[M-12] Multiple calls are executed in the same transaction in the RdpxDecayingBonds contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L70-L72 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L89-L107

Vulnerability details

Impact

Detailed description of the impact of this finding. This call is executed following another call within the same transaction. It is possible that the call never gets executed if a prior call fails permanently. This might be caused intentionally by a malicious callee.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L70-L72
  function pause() external onlyRole(DEFAULT_ADMIN_ROLE) {
    _pause();
  }
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L89-L107
  function emergencyWithdraw(
    address[] calldata tokens,
    bool transferNative,
    address payable to,
    uint256 amount,
    uint256 gas
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    _whenPaused();
    if (transferNative) {
      (bool success, ) = to.call{ value: amount, gas: gas }("");
      require(success, "RdpxReserve: transfer failed");
    }
    IERC20WithBurn token;

    for (uint256 i = 0; i < tokens.length; i++) {
      token = IERC20WithBurn(tokens[i]);
      token.safeTransfer(msg.sender, token.balanceOf(address(this)));
    }
  }

Exploit in Foundry

function testFailDecayingBonds() external payable {
        address[] memory datae = new address[](6);
        datae[0] = address(0x0000000000000000000000000000000000000001);
        datae[1] = address(0x0000000000000000000000000000000000000002);
        datae[2] = address(0x0000000000000000000000000000000000000003);
        datae[3] = address(0x0000000000000000000000000000000000000004);
        datae[4] = address(0x0000000000000000000000000000000000000005);
        datae[5] = address(0x0000000000000000000000000000000000000006);
        rdpxDecayingBonds.pause();
        rdpxDecayingBonds.emergencyWithdraw(datae, bool(true), payable(address(msg.sender)), 0, 151011896231271673956729369156563660626332691233735877138459465);
    }

Test Case in Foundry

1. Paste the function above to the end of the tests/rdpxV2-core/RdpxDecayingBondsTest.t.sol contract.
2. In the terminal run: forge test -vvvv --match-path "tests/RdpxDecayingBondsTest.t.sol" --match-test "testFailDecayingBonds"
3. Hopefully you got DOS with a successful callback and recompile.

Log

2023-08-dopex % forge test -vvvv --match-path "tests/RdpxDecayingBondsTest.t.sol" --match-test "testFailDecayingBonds"
[⠃] Compiling...
[⠊] Compiling 1 files with 0.8.19
[⠔] Solc 0.8.19 finished in 4.28s
Compiler run successful!

Running 1 test for tests/RdpxDecayingBondsTest.t.sol:RdpxDecayingBondsTest
[PASS] testFailDecayingBonds() (gas: 38331)
Traces:
  [38331] RdpxDecayingBondsTest::testFailDecayingBonds() 
    ├─ [25940] RdpxDecayingBonds::pause() 
    │   ├─ emit Paused(account: RdpxDecayingBondsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   └─ ← ()
    ├─ [5159] RdpxDecayingBonds::emergencyWithdraw([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005, 0x0000000000000000000000000000000000000006], true, 0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38, 0, 151011896231271673956729369156563660626332691233735877138459465 [1.51e62]) 
    │   ├─ [0] 0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38::fallback() 
    │   │   └─ ← ()
    │   ├─ [3000] PRECOMPILE::ecrecover(70a082310000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f) [staticcall]
    │   │   └─ ← 
    │   └─ ← "EvmError: Revert"
    └─ ← "EvmError: Revert"

Test result: ok. 1 passed; 0 failed; finished in 6.48ms

Tools Used

Mythx. VS Code. Foundry.

Recommended Mitigation Steps

If possible, refactor the code such that each transaction only executes one external call or make sure that all callees can be trusted (i.e. they're part of your own codebase).

Assessed type

DoS

[bytes032]

Invalid

[c4-pre-sort]

bytes032 marked the issue as low quality report

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid