sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

n33k - PythOracle: Keeper can commit selected favourable price from 3-second range prices #129

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

n33k

medium

PythOracle: Keeper can commit selected favourable price from 3-second range prices

Summary

Anyone can commit prices to PythOracle as long as the price is falling into the valid time range. A market user can select and commit prices in valid range that is favourable to him. This will make the protocol to have biased oracle prices and may profit the attacker and cause loses to others.

Vulnerability Detail

The price is queried from Pyth with _validateAndGetPrice. The updateData is queried offchain from Pyth's api.

    function commitRequested(uint256 versionIndex, bytes calldata updateData)
        public
        payable
        keep(KEEPER_REWARD_PREMIUM, KEEPER_BUFFER, "")
    {
        ....
        PythStructs.Price memory pythPrice = _validateAndGetPrice(versionToCommit, updateData);
        ....
        _recordPrice(versionToCommit, pythPrice);
        ....
    }

    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;

        return pyth.parsePriceFeedUpdates{value: pyth.getUpdateFee(updateDataList)}(
            updateDataList,
            idList,
            SafeCast.toUint64(oracleVersion + MIN_VALID_TIME_AFTER_VERSION),
            SafeCast.toUint64(oracleVersion + MAX_VALID_TIME_AFTER_VERSION)
        )[0].price;
    }

The updateData will be decoded, validated and extracted to produce the queried oracle price inside parsePriceFeedUpdates.

As long as the price timestamp inside updateData falls into the range between oracleVersion + MIN_VALID_TIME_AFTER_VERSION and oracleVersion + MAX_VALID_TIME_AFTER_VERSION, it is considered valid.

Currently the MIN_VALID_TIME_AFTER_VERSION is 12 and MAX_VALID_TIME_AFTER_VERSION is 15, so the range is 3 seconds. This is a large range for a perpetual protocol.

The attacker can query all the updateData in side this range from Pyth's api and select the favourable to commit.

Impact

By selecting and commiting biased price will unlikely profit the attacker. But by continuously committing biased prices, there will be accumulated effects on funding fees and user position PnLs.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

3 second is too large. Shrink range.

sherlock-admin commented 1 year ago

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

141345 commented:

l

darkart commented:

I might be wrong but Keepers are Trusted

panprog commented:

should be low because there will be a competition and if someone waits 3 seconds before commiting, it risks that commit is done by another user. Besides the 3-second impact is too low