sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

kaysoft - ETH sent to PythOracle.sol#commit function will be lost forever. #136

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

kaysoft

medium

ETH sent to PythOracle.sol#commit function will be lost forever.

Summary

The PythOracle.sol#commit function is payable but has no implementation for ETher refund and the contract does not also have a withdraw function to withdraw all Ether sent to the contract through PythOracle.sol#commit and PythOracle.sol#commitRequest functions.

Vulnerability Detail

The PythOracle.sol#commit function is payable but has no implementation for ETher refund and the contract does not also have a withdraw function to withdraw all Ether sent to the contract.

Impact

All Ether sent to the payable function PythOracle.sol#commit will be lost forever.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L164

function commit(uint256 oracleVersion, bytes calldata updateData) external payable {
        // Must be before the next requested version to commit, if it exists
        // Otherwise, try to commit it as the next request version to commit
        if (versionList.length > nextVersionIndexToCommit && oracleVersion >= versionList[nextVersionIndexToCommit]) {
            commitRequested(nextVersionIndexToCommit, updateData);
            return;
        }

        PythStructs.Price memory pythPrice = _validateAndGetPrice(oracleVersion, updateData);

        // Oracle version must be more recent than those of the most recently committed version
        if (oracleVersion <= _latestVersion) revert PythOracleVersionTooOldError();

        _recordPrice(oracleVersion, pythPrice);
        _latestVersion = oracleVersion;
    }

Tool used

Manual Review

Recommendation

Consider removing the payable keyword from the commit and commitReward functions if ETH is not required for their execution.

Duplicate of #53

sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

d

panprog commented:

low (invalid) as its user error