Asymmetry contest - qpzm's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L115-L116

Vulnerability details

Impact

ethPerDerivative must get the price of derivative in terms of ETH. However, it returns frxETHAmount / frxETHPriceInETH .

Proof of Concept

The crv pool address is as below. 2023-03-asymmetry/SfrxEth.sol at 44b5cd94ebedc187a08884a7f685e950e987261c · code-423n4/2023-03-asymmetry · GitHub

address public constant FRX_ETH_CRV_POOL_ADDRESS =
        0xa1F8A6807c402E4A15ef4EBa36528A3FED24E577;

self.balances array is as below. Vyper_contract | Address 0xa1F8A6807c402E4A15ef4EBa36528A3FED24E577 | Etherscan

self.balances = [0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE, 0x5E8422345238F34275888049021821E8E08CAa1f];

function price_oracle() of the crv pool contract returns the 1st token price denominated in the 0th token.

@external
@view
@nonreentrant('lock')
def price_oracle() -> uint256:
    amp: uint256 = self._A()
    xp: uint256[N_COINS] = self.balances
    D: uint256 = self._get_D(xp, amp)
    return self._ma_price(xp, amp, D)

Tools used

Manual analysis.

Recommended Mitigation Steps

function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    uint256 frxAmount = IsFrxEth(SFRX_ETH_ADDRESS).convertToAssets(
        10 ** 18
    );
    // This line should be fixed: / -> *
    return ((10 ** 18 * frxAmount) * IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle());
}

I submitted the gas issue by mistake, so re-submit in QA report🙏.

Cache the storage variable in for loop.

Reduce the number of sload.

// cache storage variable in stack.
uint256 _derivativeCount = derivativeCount;
for (uint256 i = 0; i < _derivativeCount; i++) {
// ...
}

• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L71
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L84
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L113
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L140
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L147
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L171
• https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L191

adjustWeight: Remove the for loop as below.

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L169

function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
        uint256 beforeWeight = weights[_derivativeIndex];
        weights[_derivativeIndex] = _weight;
        totalWeight = totalWeight - beforeWeight + _weight;
        emit WeightChange(_derivativeIndex, _weight);
    }

addDerivative: Remove the for loop as below.

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L190-L193

function addDerivative(
        address _contractAddress,
        uint256 _weight
    ) external onlyOwner {
        derivatives[derivativeCount] = IDerivative(_contractAddress);
        weights[derivativeCount] = _weight;
        derivativeCount++;
        totalWeight += _weight;
        emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
    }