sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

JuggerNaut63 - Multicall Reentrancy Exploit via Delegatecall #47

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

JuggerNaut63

High

Multicall Reentrancy Exploit via Delegatecall

Summary

The Multicall, which allows batching multiple function calls into a single transaction using delegatecall, is vulnerable to reentrancy attacks. This vulnerability can be exploited by attackers to manipulate the contract's state, potentially leading to significant financial losses.

Vulnerability Detail

The Multicall contract's multicall function uses delegatecall to execute multiple function calls within the context of the calling contract. If any of the functions called via delegatecall are not reentrancy-safe, an attacker can exploit this by making recursive calls back into the contract. This can lead to unexpected state manipulations and potential loss of funds. Specifically, the vulnerability arises because delegatecall allows the called function to modify the state of the calling contract, and without proper reentrancy guards, this can be exploited. https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/Multicall.sol#L12

Impact

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/Multicall.sol#L9-L23

Tool used

Manual Review

Recommendation

Implement reentrancy guards to prevent reentrant calls. Use OpenZeppelin's ReentrancyGuard to secure the multicall function.

+   import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

abstract contract Multicall  {
-    function multicall(bytes[] calldata data) external returns (bytes[] memory results) {
+    function multicall(bytes[] calldata data) external nonReentrant returns (bytes[] memory results) {
        results = new bytes[](data.length);
        for (uint256 i = 0; i < data.length; i++) {
            (bool success, bytes memory result) = address(this).delegatecall(data[i]);

            if (!success) {
                if (result.length == 0) revert("multicall failed");
                assembly ("memory-safe") {
                    revert(add(32, result), mload(result))
                }
            }

            results[i] = result;
        }
    }
}

PoC

VulnerableContract

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.21;

import "./Multicall.sol";

contract VulnerableContract is Multicall {
    mapping(address => uint256) public balances;

    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "Insufficient balance");
        balances[msg.sender] -= amount;
        (bool success, ) = msg.sender.call{value: amount, gas: 2300}("");
        require(success, "Transfer failed");
    }
}

ExploitTest

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.21;

import "forge-std/Test.sol";
import "../src/VulnerableContract.sol";
import "../src/Attacker.sol";

contract ExploitTest is Test {
    VulnerableContract vulnerableContract;
    Attacker attacker;

    function setUp() public {
        vulnerableContract = new VulnerableContract();
        attacker = new Attacker(address(vulnerableContract));
    }

    function testExploit() public {
        // Fund the vulnerable contract
        vm.deal(address(vulnerableContract), 10 ether);

        // Start the attack
        vm.deal(address(attacker), 1 ether);
        attacker.attack{value: 1 ether}();

        // Check if the attack was successful
        assertGt(address(attacker).balance, 1 ether, "Exploit failed");
    }
}

forge test --match-path test/ExploitTest.sol [⠊] Compiling... [⠒] Compiling 3 files with Solc 0.8.21 [⠢] Solc 0.8.21 finished in 1.98s Compiler run successful!

Ran 1 test for test/ExploitTest.sol:ExploitTest [PASS] testExploit() (gas: 43363) Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 960.20µs (278.30µs CPU time)

Ran 1 test suite in 5.48ms (960.20µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

telome commented 1 month ago

The submission fails to provide a valid example of reentrancy attack.