Reentrancy Vulnerability in WalletImpl.sol#execCalls Function

Summary

The execCalls function allows the contract owner to execute arbitrary calls without any re-entrancy guard. This lack of protection makes the contract vulnerable to re-entrancy attacks, potentially causing unexpected behavior or loss of funds.

Vulnerability Detail

The execCalls function iterates through an array of calls and executes them one by one. Since it doesn't employ any re-entrancy protection, a malicious actor could exploit this vulnerability by calling back into the execCalls function before the current call execution finishes. This could lead to undesirable side effects, such as draining the contract's balance or manipulating its state.

Impact

A successful re-entrancy attack could lead to the loss of funds, unauthorized state changes, or other unintended consequences

Code Snippet

https://github.com/0xSplits/splits-utils/blob/0dd263bf6feb0bd26b054da3cf1bb742d0bfb13e/src/WalletImpl.sol#L43-L65

Tool used

Manual Review

Recommendation

To mitigate the re-entrancy vulnerability, implement the provided nonReentrantGuard modifier and use it in the execCalls function. The modifier ensures that the function cannot be called recursively by setting the _notEntered flag before and after the function execution. Here's the implementation of the recommended nonReentrantGuard modifier:

bool private _notEntered;

modifier nonReentrantGuard() {
    require(!_notEntered, "ReentrancyGuard: reentrant call");
    _notEntered = true;
    _;
    _notEntered = false;
}

To apply the nonReentrantGuard modifier, simply add it to the execCalls function:

function execCalls(Call[] calldata calls_)
    external
    payable
    onlyOwner
    nonReentrantGuard // Add this modifier
    returns (uint256 blockNumber, bytes[] memory returnData)
{
    // Function implementation
}

Duplicate of #49

sherlock-audit / 2023-04-splits-judging

0xPkhatri - Reentrancy Vulnerability in WalletImpl.sol#execCalls Function #100