Open Dollar - Al-Qa-qa's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/ODProxy.sol#L26

Vulnerability details

Impact

the execute function in the ODProxy contract has a payable modifier, which implies that it can receive Ether. However, the function utilizes delegatecall to execute code in another contract (Action contracts like BasicAction), which does not forward the Ether sent to the execute function. As a result, any Ether sent to this function becomes trapped within the contract.

// ODProxy::execute
// @audit [M: payable function with no handled ETH]
function execute(
    address _target,
    bytes memory _data
) external payable onlyOwner returns (bytes memory _response) {
    if (_target == address(0)) revert TargetAddressRequired();

    bool _succeeded;
    (_succeeded, _response) = _target.delegatecall(_data);

    if (!_succeeded) {
        revert TargetCallFailed(_response);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Since The protocol doesn't depend on the native ETH coin, the payable modifier should be removed from the execute function in the ODProxy contract.

Assessed type

ETH-Transfer