code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

TimelockControllerEmergency: The sent ether may be locked in the OwnerProxy contract #27

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/governance/TimelockControllerEmergency.sol#L351-L362

Vulnerability details

Impact

The _call function in the TimelockControllerEmergency contract will send ether to the OwnerProxy contract, and the OwnerProxy contract will delegatecall the script contract. The two existing script contracts will neither use ether nor withdraw ether, which will cause the sent ether to be locked in the OwnerProxy contract.

    function _call(
        bytes32 id,
        uint256 index,
        address target,
        uint256 value,
        bytes calldata data
    ) private {
        (bool success, ) = target.call{ value: value }(data);
        require(success, "TimelockController: underlying transaction reverted");

        emit CallExecuted(id, index, target, value, data);
    }

Proof of Concept

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/governance/TimelockControllerEmergency.sol#L351-L362

Tools Used

None

Recommended Mitigation Steps

Consider adding the function of withdrawing ether to the OwnerProxy contract, or adding a script contract for withdrawing ether

obatirou commented 2 years ago

TimelockControllerEmergency: The sent ether may be locked in the OwnerProxy contract (disputed)

We can set the value to zero to not send ether. And if we made a mistake, we can create a script to send the ether back.