Dopex - mussucal's results

Lines of code

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

Vulnerability details

Impact

Calling 'rdpxV2Core.bondWithDelegate()' with extra WETH and then calling 'rdpxV2Core.withdraw()' to withdraw the unused WETH. After which 'rdpxV2Core.bond()' or 'rdpxV2Core.bondWithDelegate()' is called. The WETH backing reserve value is not correct.

Proof of Concept

In 'rdpxV2Core.withdraw()', the public variable totalWethDelegated is not reduced. Calling 'rdpxV2Core.sync()' after 'rdpxV2Core.withdraw()' causes arithmetic overflow in line 1002.

A modified test case from tests/rdpxV2-core/Unit.t.sol/testBondWithDelegate()

function testBondWithDelegateWithExtraWeth() public {
    // assert token reserves
    (, uint256 rdpxBalance, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    (, uint256 wethBalance, ) = rdpxV2Core.getReserveTokenInfo("WETH");

    /// add to delegate with different fees
    uint256 delegateId = rdpxV2Core.addToDelegate(50 * 1e18, 10 * 1e8);

    // test bond with delegate
    uint256[] memory _amounts = new uint256[](1);
    uint256[] memory _delegateIds = new uint256[](1);
    _delegateIds[0] = 0;
    _amounts[0] = 1 * 1e18;

    // address 1 bonds
    /// check user balance
    uint256 userRdpxBalance = rdpx.balanceOf(address(1));
    uint256 userWethBalance = weth.balanceOf(address(1));

    (uint256 rdpxRequired, uint256 wethRequired) = rdpxV2Core.calculateBondCost(
      1 * 1e18,
      0
    );

    vm.prank(address(1));
    rdpx.approve(address(rdpxV2Core), 2 * 1e18);
    vm.prank(address(1), address(1));
    (uint256 userAmount, uint256[] memory delegateAmounts) = rdpxV2Core
      .bondWithDelegate(address(1), _amounts, _delegateIds, 0);

    // check the correct amounts have been transferred
    assertEq(rdpx.balanceOf(address(1)), userRdpxBalance - rdpxRequired);
    assertEq(weth.balanceOf(address(1)), userWethBalance);
    uint256 amount;
    uint256 activeCollateral;
    (, amount, , activeCollateral) = rdpxV2Core.delegates(delegateId);

    assertEq(amount, 50 * 1e18);
    assertEq(activeCollateral, wethRequired);

    rdpxV2Core.withdraw(_delegateIds[0]);                                 //Comment these two lines for the
    rdpxV2Core.sync();                                                    //test to pass

    assertEq(
      rdpxV2ReceiptToken.balanceOf(address(rdpxV2Core)),
      userAmount + delegateAmounts[0]
    );

    // assert token reserves
    (, rdpxBalance, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    (, wethBalance, ) = rdpxV2Core.getReserveTokenInfo("WETH");

    assertEq(rdpxBalance, 1275 * 1e15);
    assertGt(wethBalance, 245 * 1e15);
  }

Tools Used

Manual review and foundry.

Recommended Mitigation Steps

'totalWethDelegated' needs to be decremented by the amount of unused WETH withdrawn in 'rdpxV2Core.withdraw()'.

Assessed type

Error