GimelSec - TimeLock.execute lacks payable

[sherlock-admin]

GimelSec

high

TimeLock.execute lacks payable

Summary

TimeLock.execute lacks payable. If _value in TimeLock.execute is not zero, it could always revert.

Vulnerability Detail

TimeLock.execute lacks payable. The caller cannot send the value.
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/TimeLock.sol#L113

    function execute(
        address _target,
        uint256 _value,
        string calldata _func,
        bytes calldata _data,
        uint256 _timestamp
    ) external onlyOwner returns (bytes memory) {
        …

        // call target
        (bool ok, bytes memory res) = _target.call{value: _value}(data);
        …
    }

And the contract is modified from https://solidity-by-example.org/app/time-lock/ . The example code has the payable receive function. But TimeLock doesn’t have one.

Impact

TimeLock.execute cannot work if _value != 0.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/TimeLock.sol#L113

Tool used

Manual Review

Recommendation

Add payable on TimeLock.execute. Also add a check to ensure msg.value == _value.

    function execute(
        address _target,
        uint256 _value,
        string calldata _func,
        bytes calldata _data,
        uint256 _timestamp
    ) external payable onlyOwner returns (bytes memory) {
        …

        // call target
        (bool ok, bytes memory res) = _target.call{value: _value}(data);
        …
    }

[3xHarry]

valid issue

[Jiaren-tang]

Escalate for 10 USDC. Valid low. Definitely not medium. there is no admin function that needs msg.value.

[sherlock-admin]

Escalate for 10 USDC. Valid low. Definitely not medium. there is no admin function that needs msg.value.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[3xHarry]

fix PR: Y2K-Finance/Earthquake#134

[dmitriia]

execute does use native funds.

Update: but it looks like currently there is no onlyTimeLocker function that uses native funds (and there is whitelistController() that not onlyTimeLocker, but can be run by TimeLock, but it doesn't use native funds either), so the issue is best classified as a valid low.

[hrishibhat]

Escalation accepted

Valid low
Based on the above comments considering this issue as a valid low

[sherlock-admin]

Escalation accepted

Valid low
Based on the above comments considering this issue as a valid low

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.