sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

0xVinylDavyl - The contract calls pyth.parsePriceFeedUpdates with a value based on IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length. If this value exceeds the block gas limit, the transaction will fail. #18

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

0xVinylDavyl

high

The contract calls pyth.parsePriceFeedUpdates with a value based on IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length. If this value exceeds the block gas limit, the transaction will fail.

Summary

The "Potential Gas Limit Issues" concern the contract's interaction with an external contract (pyth) using potentially variable amounts of gas. If this interaction consumes excessive gas and exceeds the block gas limit, it could render the contract unusable.

Vulnerability Detail

The vulnerability arises from the contract's reliance on external contract interactions without taking into account the gas consumption of these interactions. In particular, the contract calls pyth.parsePriceFeedUpdates with a value based on IPythStaticFee(address(pyth)).singleUpdateFeeInWei() * idList.length. If this value exceeds the block gas limit, the transaction will fail.

Impact

Exceeding the block gas limit can result in the failure of the transaction, causing it to revert. This can make the contract unusable for certain operations, especially when interacting with external contracts that consume significant gas.

Code Snippet

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

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

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

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

Manual Review

Recommendation

To mitigate the potential gas limit issues when interacting with external contracts, follow these recommended steps:

  1. Use Gas Stipends: Consider using the "gas stipends" feature introduced in Solidity 0.8.9 to control the gas allocated for external calls. Gas stipends allow you to specify the maximum amount of gas to be forwarded to an external contract, preventing excessive gas consumption.

    // Import the Gas Stipend feature from Solidity 0.8.9
    import "@openzeppelin/contracts/access/GasStipendControl.sol";
    
    // Inside the contract
    using GasStipendControl for uint256;
  2. Limit Gas Consumption: Modify the interaction with the pyth contract to use gas stipends to limit gas consumption. Ensure that the gas stipend provided is reasonable and does not exceed the block gas limit.

    // Limit gas consumption using a stipend
    uint256 stipend = 100000;  // Adjust the stipend value as needed
    return pyth.parsePriceFeedUpdates{gas: stipend, value: ...}(
       // ...
    )[0].price;

By implementing these recommendations and using gas stipends, you can control the gas allocated for external calls, preventing the contract from exceeding the block gas limit and ensuring its usability.

sherlock-admin commented 11 months ago

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

panprog commented:

invalid because it has nothing to do with gas, just a payment for price feed processing

n33k commented:

invalid, ai generated groundless report

darkart commented:

The same as 14

polarzero commented:

Invalid. Again, this would not cause any issue in the functioning of the protocol, but rather waste gas for the user.