code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Function RdpxV2Core#withdraw() lets delegate owners withdraw their unused WETH. However, withdrawn amount is not deducted from totalWethDelegated, which causes WETH asset reserve tracked improperly. The impacts could be:

Function sync gets reverted when totalWethDelegated is greater than token balance
Due to wrong accounting asset reserve from sync() function, other logics get affected: a. lowerDepeg could get reverted because of underflowed at line reserveAsset[reservesIndex["WETH"]].tokenBalance -= _wethAmount; b. function provideFunding could get reverted because of underflowed ...

So far, many functions would get DoS

Proof of Concept

diff --git a/tests/rdpxV2-core/Unit.t.sol b/tests/rdpxV2-core/Unit.t.sol
index e11c284..710e5b3 100644
--- a/tests/rdpxV2-core/Unit.t.sol
+++ b/tests/rdpxV2-core/Unit.t.sol

+  function testWithdraw_NotDecreaseTotalDelegated() public {
+    rdpxV2Core.addToDelegate(1 * 1e18, 10e8);
+    rdpxV2Core.withdraw(0);
+
+    // sync() get revert due to underflowed
+    vm.expectRevert();
+    rdpxV2Core.sync();
+
+    dpxETH.approve(address(curvePool), 200 * 1e18);
+    dpxETH.mint(address(this), 200 * 1e18);
+    address coin0 = IStableSwap(curvePool).coins(0);
+    if (coin0 == address(weth)) {
+      IStableSwap(curvePool).exchange(1, 0, 110 * 1e18, 0);
+    } else {
+      IStableSwap(curvePool).exchange(0, 1, 110 * 1e18, 0);
+    }
+
+    rdpxV2Core.bond(5 * 1e18, 0, address(1));
+    rdpxV2Core.sync();
+    dpxEthPriceOracle.updateDpxEthPrice(98137847);
+    (, uint256 wethReserve1, ) = rdpxV2Core.getReserveTokenInfo("WETH");
+
+    vm.expectRevert();
+    rdpxV2Core.lowerDepeg(0, 1e18, 0, 0);
+  }
+
   function testUpperDepeg() public {
     // test upper depeg while price is lower than depeg
     vm.expectRevert(

Tools Used

Foundry, Manual review

Recommended Mitigation Steps

Add totalWethDelegated -= totalWethDelegated += _amount; to function withdraw

Assessed type

Other

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #2186

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report