Dopex - halden's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1002

Vulnerability details

Impact

Storage variable totalWethDelegated can be easily manipulated if malicious user deposit big amount of weth and than imidieatly withdraw through withdraw functions. When normal user deposit weth they call addToDelegate function and deposited amount is added to totalWethDelegated: totalWethDelegated += _amount. When user bond with delegate he call bondWithDelegate and only weth required is substracted from totalWethDelegated totalWethDelegated -= wethRequired

The storage variable totalWethDelegated can be easily manipulated if a malicious user deposits a large amount of WETH and immediately withdraws it through the withdraw function. When a normal user deposits WETH, they call the addToDelegate function, and the deposited amount is added to totalWethDelegated: totalWethDelegated += _amount. When a user bonds with a delegate, they call the bondWithDelegate function and only the required WETH is subtracted from totalWethDelegated: totalWethDelegated -= wethRequired.

Unfortunatly, when user withdraw your weth through withdraw function variable totalWethDelegated is not substracted but balance of contract will deacrease because eth are transfered to user: IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn). This open vulnarability because totalWethDelegated will be bigger than actual balance and function sync will revert during arithmetic underflow

Unfortunately, when a user withdraws their WETH through the withdraw function, the variable totalWethDelegated is not subtracted. Instead, the contract's balance decreases because WETH is transferred to the user using this line of code: IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn). This creates a vulnerability because totalWethDelegated will be larger than the actual balance and the sync function will revert due to an arithmetic underflow. balance = balance - totalWethDelegated

Proof of Concept

Test:

  function testManipulateTotalWethDelegated() public {
    // add 2weth
    uint delegateId = rdpxV2Core.addToDelegate(2 * 1e18, 10e8);

    // withdraw 2weth
    rdpxV2Core.withdraw(delegateId);

    // rdpxV2Core.sync(); -> this will revert if it is called.

    uint256 contractBalance = weth.balanceOf(address(rdpxV2Core));
    
    uint256 totalWethDelegated = rdpxV2Core.totalWethDelegated();

    // Revert
    assertEq(contractBalance, totalWethDelegated);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Substract amountWithdrawn from totalWethDelegated when withdraw function is called.

Assessed type

Under/Overflow