DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: medium
Valid

[M-2] AggregatorV2V3 interface hardcoded in `KeeperProxy.sol` making the `_validatePrice` always revert on Avalanche chain

Description

In the keeperProxy::Initialize function the AggregatorV2V3 interface is hardcoded and to its Arbitrum address which is used check the L2 sequencer status in KeeperProxy::_validatePrice function but if the same code is deployed on Avalanche chain then the _validatePrice function always revert blocking the run and runAction functions from being executed.

Impact

The Readme states that the protocol is expected to be deployed on Arbitrum and Avalanche chains but it is not possible to deploy the protocol on Avalanche chain as the AggregatorV2V3 interface is hardcoded to Arbitrum address making the protocol unusable on Avalanche chain

Proof of Concept

  1. HardCoded sequencerUptime Feed to Arbitrum feed address

function initialize() external initializer {
__Ownable2Step_init();
@> sequencerUptimeFeed = AggregatorV2V3Interface(0xFdB631F5EE196F0ed6FAa767959853A9F217697D);
}
  1. The specified block checks if L2 sequencer is up but doesn't check the block.chainId of the chain in which it is being executed , on Avalanche the Sequencer address doesn't point to the same L2 sequencer address instead it points to an EOA meaning the function always reverts on Avalanche chain

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
-----> // L2 Sequencer check
| T (
| H /*uint80 roundID*/
| I ,
| S int256 answer,
| uint256 startedAt,
| /*uint256 updatedAt*/
| B ,
| L /*uint80 answeredInRound*/
| O ) = AggregatorV2V3Interface(sequencerUptimeFeed).latestRoundData();
| C bool isSequencerUp = answer == 0;
| K require(isSequencerUp, "sequencer is down");
| // Make sure the grace period has passed after the sequencer is back up.
| uint256 timeSinceUp = block.timestamp - startedAt;
| require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over");
|---->
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}

Tools Used

Manual Review

Recommendations

In _validatefunction add a statement to verify block.chainId i.e if Arbitrum do the L2 sequencer check , if Avalanche skip that block this way the same keeper code can be used in both the chains

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_Avalanche_has_no_sequencer

Likelihood: High, run and runNextAction will revert. Impact: Low, any deposit will be retrieve thanks to cancelFlow.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!