Dopex - lanrebayode77's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L200-L203

Vulnerability details

Impact

PerpetualAtlanticVault.settle() is susceptible to frontrunning which could cause the function to revert due to a call made to PerpetualAtlanticVaultLP.subtractLoss().

Proof of Concept

After transferring WETH from PerpetualAtlanticVaultLP to the RdpxV2Core, and RDPX is transferred from RdpxV2Core to the PerpetualAtlanticVaultLP, PerpetualAtlanticVaultLP.subtractLoss() is triggered to deduct the transferred WETH from _totalCollateral.

The problem with PerpetualAtlanticVaultLP.subtractLoss() function is the check that requires collateral.balanceOf(address(this)) == _totalCollateral - loss, this check can be easily made to fail by sending dust(little) amount of WETH to the PerpetualAtlanticVaultLP contract address directly which will not be accounted for in _totalCollateral.

require(
      collateral.balanceOf(address(this)) == _totalCollateral - loss,
      "Not enough collateral was sent out"
    );

Tools Used

Manual review.

Recommended Mitigation Steps

Consider changing the equality sign to less or equal.

require(
      collateral.balanceOf(address(this)) <= _totalCollateral - loss,
      "Not enough collateral was sent out"
    );

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L159-L174

Vulnerability details

Impact

Whenever shares are redeemed in PerpetualAtlanticVaultLP, there's an accounting deficit that causes a revert in PerpetualAtlanticVaultLP.addProceeds(), leading to a denial of service in multiple functions that makes calls to PerpetualAtlanticVaultLP.addProceeds() through PerpetualAtlanticVault.updateFunding() like; PerpetualAtlanticVault.purchase(), PerpetualAtlanticVault.settle(), PerpetualAtlanticVaultLP.deposit(), and PerpetualAtlanticVaultLP.redeem()

Proof of Concept

1. A user makes deposit to mint shares through PerpetualAtlanticVaultLP.deposit() where funds is transferred from user to the contract and _totalCollatera is updated(increamented). https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L128-L132

 // Need to transfer before minting or ERC777s could reenter.
    collateral.transferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

    _totalCollateral += assets;

2. When shares are redeemed, funds(collateral and rpdx) are transferred to user, _rdpxCollateral is updated(decremented) but _totalCollateral is not. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L162-L175

 _rdpxCollateral -= rdpxAmount;

    beforeWithdraw(assets, shares);

    _burn(owner, shares);

    collateral.transfer(receiver, assets);

    IERC20WithBurn(rdpx).safeTransfer(receiver, rdpxAmount);

    emit Withdraw(msg.sender, receiver, owner, assets, shares);

3. The problem here is, in PerpetualAtlanticVaultLP.addProceeds() a check is done to assert that balanceOf Collateral is equal or greater than _totalCollateral + proceeds, this will NEVER hold if shares have been redeemed. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L191-L194

require(
      collateral.balanceOf(address(this)) >= _totalCollateral + proceeds,
      "Not enough collateral token was sent"
    );

Instance:

--Alice makes a deposit of 100 collateral token to get equivalent shares, this increases _totalCollateral by 100. --Alice redeems 50% of her shares to get 50 collateral token transferred back to her by the contract, reducing the contract balance by 50 but no corresponding decrease in _totalCollateral. --So whenever collateral.balanceOf(address(this)) >= _totalCollateral + proceeds is checked, _totalCollateral will still hold 100+, so the check always fails. --The ripple effect is that all function that relies on the check fails.

Tools Used

Manual review.

Recommended Mitigation Steps

Update _totalCollateral after transferring collateral to the redeemer.

_totalCollateral -= assets;

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L975-L990

Vulnerability details

Impact

RdpxV2Core.sync() will fail if a whale delegate withdraws a large amount of WETH or WETH reserve balance gets updated with a wrong value when a smaller withdrawal is made due to un-updated totalWethDelegated in the withdraw() function.

Another problem this will cause is that RdpxV2Core.bond() and RdpxV2Core.bondWithDelegate() will revert because they both make a call to ReLPContrac.reLp() function which calls the RdpxV2Core.sync(), this will also cause both functions to fail if isReLPActive is true.

Proof of Concept

1. When a user calls addDelegate() and deposits WETH, totalWethDelegated is incremented with the deposited amount. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L963-L964

// add amount to total weth delegated
    totalWethDelegated += _amount;

2. When another user calls bondWithDelegate(), a call is made to _bondWithDelegate() that decrements the totalWethDelegated by the amount of WETH required for the bonding. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L719-L720

 // update total weth delegated
    totalWethDelegated -= wethRequired;

3. However, when a delegate calls withdraw(), available WETH is sent to the delegate without updating(decrement) the totalWethDelegated by the withdrawn amount.

The problem is that, when syn() is called, it checks for WETH and decrements the balance by totalWethDelegated. If a withdrawal was made, the contract WETH balance will reduce by the withdrawn amount and the calculated balance will give wrong value, in a case where a whale delegate makes a huge withdrawal the function will fail due to an overflow exception error.

Instance:

--Alice calls addDelegate() to deposit 1000WETH, totalWethDelegated increased by 1000 --10WETH was used for bonding so totalWethDelegated reduces to 990WETH --She withdrew 990WETH but totalWethDelegated remains the same --syn() is called and WETH balance was 300WETH, but balance = balance - totalWethDelegated is 300 - 990(instead of 300 - 0 = 300) which will cause the entire function to revert due to solidity prevention of overflow. -- If the withdrawn amount is lower(let's say 200), WETH balance will be updated with a wrong value of 1100 - 200 = 900 instead of 1100 - 0 = 1100 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1001-L1004

if (weth == reserveAsset[i].tokenAddress) {
        balance = balance - totalWethDelegated;
      }
      reserveAsset[i].tokenBalance = balance;
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Update totalWethDelegated during withdrawals

totalWethDelegated -= wethRequired;

Assessed type

Math