sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

Daniel_MetaTrust - Potential Lock Ether Forever #21

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

Daniel_MetaTrust

medium

Potential Lock Ether Forever

Summary

In the PythOracle contract, the commit function is used to commit the price to a non-requested version at a cost of Ether, but lacks checking if the msg.value equals to the expected usage.

Vulnerability Detail

In the commit function of the PythOracle contract, , the amount of Ether used is IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length when invoking the pyth.parsePriceFeedUpdates function from the _validateAndGetPrice function. However, there is no checking to validate if the msg.value equals to IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length, which results in the transaction reverting if the msg.value less than expected or the redundant Ether locked into the PythOracle forever if the msg.value is greater than expected since there is function to withdraw redundant Ether.

The same scenario happens for the commitRequested function.

Impact

Redundant Ether locked into the PythOracle contract forever when the msg.value sent to the commit contract is greater than expected.

Code Snippet

Function commit calls the _validateAndGetPrice function.

    function commit(uint256 versionIndex, 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 > versionIndex &&                // must be a requested version
            versionIndex >= nextVersionIndexToCommit &&         // must be the next (or later) requested version
            oracleVersion == versionList[versionIndex]          // must be the corresponding timestamp
        ) {
            commitRequested(versionIndex, updateData);
            return;
        }

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

Function _validateAndGetPrice calls the parsePriceFeedUpdates function with a cost of IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length Ether :

    function _validateAndGetPrice(uint256 oracleVersion, bytes calldata updateData) private returns (PythStructs.Price memory price) {
        bytes[] memory updateDataList = new bytes[](1);
        updateDataList[0] = updateData;
        bytes32[] memory idList = new bytes32[](1);
        idList[0] = id;

        // Limit the value passed in the single update fee * number of updates to prevent packing the update data
        // with extra updates to increase the keeper fee. When Pyth updates their fee calculations
        // we will need to modify this to account for the new fee logic.
        return pyth.parsePriceFeedUpdates{value: IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length}(
            updateDataList,
            idList,
            SafeCast.toUint64(oracleVersion + MIN_VALID_TIME_AFTER_VERSION),
            SafeCast.toUint64(oracleVersion + MAX_VALID_TIME_AFTER_VERSION)
        )[0].price;
    }

Tool used

Vscode, Manual Review

Recommendation

Checking if the msg.value equals to the IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length or returning redundant Ether back to the caller.

sherlock-admin commented 11 months ago

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

panprog commented:

invalid because this is user mistake which is not a valid issue by sherlock rules, similar finding was also judged invalid in main contest

n33k commented:

invalid, user error

polarzero commented:

Invalid. Users sending ETH accidentally just because the contract allows it (even if it's through a function call) is not a valid medium/high issue.