Dopex - gumgumzum's results

Lines of code

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

Vulnerability details

Impact

totalWethDelegated is not updated after withdrawing unused WETH leading incorrect WETH reserve balance after a sync or its failure due to an arithmetic underflow.

Proof of Concept

Test

  function testSync() public {
    weth.transfer(address(rdpxV2Core), 10 ether);
    
    uint256 rdpxV2CoreWethBalance = weth.balanceOf(address(rdpxV2Core));
    
    weth.transfer(address(vm.addr(8)), rdpxV2CoreWethBalance * 2);

    vm.startPrank(vm.addr(8));

    weth.approve(address(rdpxV2Core), rdpxV2CoreWethBalance * 2);

    rdpxV2Core.sync();
    uint256 delegateId = rdpxV2Core.addToDelegate(rdpxV2CoreWethBalance * 2, 2e8);
    rdpxV2Core.withdraw(delegateId);

    vm.stopPrank();

    rdpxV2Core.sync();
  }

Results

Running 1 test for tests/rdpxV2-core/Unit.t.sol:Unit
[FAIL. Reason: Arithmetic over/underflow] testSync() (gas: 311989)
Test result: FAILED. 0 passed; 1 failed; finished in 1.48s

Tools Used

Manual Review

Recommended Mitigation Steps

Decrement totalWethDelegated when withdrawing unused WETH

  function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

    totalWethDelegated -= amountWithdrawn;
    
    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

Assessed type

Math