Dopex - ether_sky's results

Lines of code

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

Vulnerability details

Impact

One user add delegate by calling addToDelegate() function with some weth. After some time, that user withdraw the remaining weth by calling withdraw() function. The problem here is that totalWethDelegated variable is updated only in addToDelegate() function not in withdraw(). And call sync() function. Then reserveAsset[reservesIndex["WETH"]].tokenBalance is updated incorrectly and once it has done, it is not revertable.

Proof of Concept

please add below function to 'tests/rdpxV2-core/Unit.t.sol' and run 'forge test'.

 function testTotalWethDelegated() public {
    rdpxV2Core.bond(10 * 1e18, 0, address(this));

    uint256 userBalanceBeforeDelegate = weth.balanceOf(address(rdpxV2Core));
    (, uint256 wethReserveBeforeDelegate, ) = rdpxV2Core
        .getReserveTokenInfo("WETH");
    // reserveAsset[reservesIndex["WETH"]].tokenBalance > 1e18
    assertGt(wethReserveBeforeDelegate, 1e18);

    rdpxV2Core.addToDelegate(1 * 1e18, 10e8);
    uint256 userBalanceAfterDelegate = weth.balanceOf(address(rdpxV2Core));
    (, uint256 wethReserveAfterDelegate, ) = rdpxV2Core.getReserveTokenInfo(
        "WETH"
    );

    // real balance changes, but reserveAsset[reservesIndex["WETH"]].tokenBalance doesn't change
    assertEq(userBalanceAfterDelegate, userBalanceBeforeDelegate + 1e18);
    assertEq(wethReserveBeforeDelegate, wethReserveAfterDelegate);

    rdpxV2Core.withdraw(0);
    (, uint256 wethReserveAfterWithdraw, ) = rdpxV2Core.getReserveTokenInfo(
        "WETH"
    );
    // reserveAsset[reservesIndex["WETH"]].tokenBalance doesn't change
    assertEq(wethReserveBeforeDelegate, wethReserveAfterWithdraw);

    rdpxV2Core.sync();
    (, uint256 wethReserveAfterSync, ) = rdpxV2Core.getReserveTokenInfo(
        "WETH"
    );
    /** 
        1e18 was removed from 'reserveAsset[reservesIndex["WETH"]].tokenBalance' because 'totalWethDelegated' didn't change when 'withdraw' delegate.
        This is obviously error.
    */
    assertEq(wethReserveAfterSync, wethReserveAfterWithdraw - 1e18);
}

Tools Used

Manual audit

Recommended Mitigation Steps

Should add below code to withdraw() function.

totalWethDelegated -= amountWithdrawn;

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L273-L275

Vulnerability details

Impact

If the rDPX price is higher than EDH (if possible), the expected amount of token A cannot be exchanged because this incorrect value is higher than the correct value. This means it violates the reLP logic.

Since rDPX prices are usually much lower than EDH, this incorrect value is less than the correct value, so you can swap less than you expect.

Proof of Concept

tokenBAmount = tokenAAmount * tokenAPrice / 1e8 tokenAAmount = tokenBAmount * 1e8 / tokenAPrice

For example: tokenAPrice = 1e6 (i.e. 1 tokenB = 100 tokenA ) And we want to swap 1e18 tokenB for tokenA. (We are gonna ignore slippage.) The calculated value in code : tokenA = tokenB * tokenAPrice / 1e8 = 1e18 * 1e6 / 1e8 = 1e16 This means that token A is more valuable than tokenB. Actual value will be tokenB * 1e8 / tokenAPrice = 1e20

Tools Used

Manual audit

Recommended Mitigation Steps

  mintokenAAmount = 
      (((amountB / 2) * 1e8) / tokenAInfo.tokenAPrice) -
      (((amountB / 2) * 1e8) / tokenAInfo.tokenAPrice) * slippageTolerance / 1e8;

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

When the 'nextFundingPaymentTimestamp()' is less than 'block.timestamp', we can't bond due to we get 'Arithmetic over/underflow'. As far as I understood, the 'latestFundingPaymentPointer' doesn't be updated automatically. In order to bond, the 'latestFundingPaymentPointer' should be updated in Vault.

Proof of Concept

Please add below function to 'tests/rdpxV2-core/Unit.t.sol' and run 'forge test'.

function testBondInNextEpoch() public {
    vault.updateFundingPaymentPointer();
    // move to future epoch, 'latestFundingPaymentPointer' didn't be updated yet.
    skip(2 * vault.fundingDuration());
    // failed with 'Arithmetic over/underflow' error.
    rdpxV2Core.bond(1 * 1e18, 0, address(this));
}

Tools Used

Manual audit

Recommended Mitigation Steps

Add below line to 'IPerpetualAtlanticVault'.

function updateFundingPaymentPointer() external;

And add below lines before calling calculateBondCost() function in the core.

IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
        .updateFundingPaymentPointer();

Assessed type

Under/Overflow