bytes032 - Missing checks for whether Arbitrum Sequencer is active

[sherlock-admin]

bytes032

medium

Missing checks for whether Arbitrum Sequencer is active

Summary

Missing checks for whether Arbitrum Sequencer is active

Vulnerability Detail

the onchain deployment context is changed, in prev contest the protocol only attemps to deploy the code to ethereum while in the current contest

the protocol intends to deploy to arbtrium as well!

Chainlink recommends that users using price oracles, check whether the Arbitrum sequencer is active

https://docs.chain.link/data-feeds#l2-sequencer-uptime-feeds

If the sequencer goes down, the index oracles may have stale prices, since L2-submitted transactions (i.e. by the aggregating oracles) will not be processed.

Impact

Stale prices, e.g. if USDC were to de-peg while the sequencer is offline, stale price is used and can result in false liquidation or over-borrowing.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/oracle/JOJOOracleAdaptor.sol#L26-L35

    function getAssetPrice() external view override returns (uint256) {
        /*uint80 roundID*/
        (, int256 price,, uint256 updatedAt,) = IChainLinkAggregator(chainlink).latestRoundData();
        (, int256 USDCPrice,, uint256 USDCUpdatedAt,) = IChainLinkAggregator(USDCSource).latestRoundData();

        require(block.timestamp - updatedAt <= heartbeatInterval, "ORACLE_HEARTBEAT_FAILED");
        require(block.timestamp - USDCUpdatedAt <= heartbeatInterval, "USDC_ORACLE_HEARTBEAT_FAILED");
        uint256 tokenPrice = (SafeCast.toUint256(price) * 1e8) / SafeCast.toUint256(USDCPrice);
        return tokenPrice * JOJOConstant.ONE / decimalsCorrection;
    }

Tool used

Manual Review

Recommendation

Use sequencer oracle to determine whether the sequencer is offline or not, and don't allow orders to be executed while the sequencer is offline.

[Trumpero]

During the time when the sequencer is down, transactions should be reverted to prevent users from experiencing unexpected prices once the sequencer is up again. Therefore, I believe this issue should be classified as medium severity.

[arbitrary-cr0]

Escalate for 10 USDC

This issue is invalid because:

1. It misunderstands the behavior of Arbitrum and its sequencer when the sequencer is down. If/when Arbitrum's sequencer goes down, no transactions will be able to be processed that originate on the L2 itself. This means that any user that attempts to interact with JOJO directly from L2 will be unable to do so. EOAs would be able to force include a transaction from the DelayedInbox on L1 - however, force including a transaction can only occur once a transaction has sat in the DelayedInbox for 24 hours (reference). Therefore, the attack relies on the sequencer being down for over 24 hours, which is unlikely. Additionally, the behavior of the sequencer when it is down/misbehaving is undefined, meaning the ordering of transactions may not give the attacker the advantage they were seeking if the sequencer starts processing transactions before the attacker's lone transaction can be force-included. This makes the attack very difficult to perform successfully.

2. Its stated impact is infeasible. Regarding liquidation, if the price before the sequencer went offline resulted in positions that could be liquidated, then they should be liquidated regardless (even though it would be very difficult to liquidate an account while the sequencer is down based on the reasons given in 1). Furthermore, the given example for impact mentions USDC de-pegging while the sequencer is offline, but the on-chain value being used will not have de-pegged, meaning users would not be able to be liquidated for the duration that the sequencer is offline. Also, the issue mentions over-borrowing, but again users will not be able to borrow easily while the sequencer is offline AND all msg.sender addresses that initiate an L1-to-L2 transaction will have their address aliased, so users wouldn't even be able to submit orders because the msg.sender will not correspond to the address of their account on L2.

Additionally, this remark on the proposed recommendation will not do anything to protect users as soon as the sequencer is up and running again:

During the time when the sequencer is down, transactions should be reverted to prevent users from experiencing unexpected prices once the sequencer is up again. Therefore, I believe this issue should be classified as medium severity.

[arbitrary-sparrow]

Escalate for 10 USDC

Reposting @arbitrary-cr0 so it gets picked up by the bot:

This issue is invalid because:

1. It misunderstands the behavior of Arbitrum and its sequencer when the sequencer is down. If/when Arbitrum's sequencer goes down, no transactions will be able to be processed that originate on the L2 itself. This means that any user that attempts to interact with JOJO directly from L2 will be unable to do so. EOAs would be able to force include a transaction from the DelayedInbox on L1 - however, force including a transaction can only occur once a transaction has sat in the DelayedInbox for 24 hours (reference). Therefore, the attack relies on the sequencer being down for over 24 hours, which is unlikely. Additionally, the behavior of the sequencer when it is down/misbehaving is undefined, meaning the ordering of transactions may not give the attacker the advantage they were seeking if the sequencer starts processing transactions before the attacker's lone transaction can be force-included. This makes the attack very difficult to perform successfully.

2. Its stated impact is infeasible. Regarding liquidation, if the price before the sequencer went offline resulted in positions that could be liquidated, then they should be liquidated regardless (even though it would be very difficult to liquidate an account while the sequencer is down based on the reasons given in 1). Furthermore, the given example for impact mentions USDC de-pegging while the sequencer is offline, but the on-chain value being used will not have de-pegged, meaning users would not be able to be liquidated for the duration that the sequencer is offline. Also, the issue mentions over-borrowing, but again users will not be able to borrow easily while the sequencer is offline AND all msg.sender addresses that initiate an L1-to-L2 transaction will have their address aliased, so users wouldn't even be able to submit orders because the msg.sender will not correspond to the address of their account on L2.

Additionally, this remark on the proposed recommendation will not do anything to protect users as soon as the sequencer is up and running again:

During the time when the sequencer is down, transactions should be reverted to prevent users from experiencing unexpected prices once the sequencer is up again. Therefore, I believe this issue should be classified as medium severity.

[sherlock-admin]

Escalate for 10 USDC

Reposting @arbitrary-cr0 so it gets picked up by the bot:

This issue is invalid because:

1. It misunderstands the behavior of Arbitrum and its sequencer when the sequencer is down. If/when Arbitrum's sequencer goes down, no transactions will be able to be processed that originate on the L2 itself. This means that any user that attempts to interact with JOJO directly from L2 will be unable to do so. EOAs would be able to force include a transaction from the DelayedInbox on L1 - however, force including a transaction can only occur once a transaction has sat in the DelayedInbox for 24 hours (reference). Therefore, the attack relies on the sequencer being down for over 24 hours, which is unlikely. Additionally, the behavior of the sequencer when it is down/misbehaving is undefined, meaning the ordering of transactions may not give the attacker the advantage they were seeking if the sequencer starts processing transactions before the attacker's lone transaction can be force-included. This makes the attack very difficult to perform successfully.

2. Its stated impact is infeasible. Regarding liquidation, if the price before the sequencer went offline resulted in positions that could be liquidated, then they should be liquidated regardless (even though it would be very difficult to liquidate an account while the sequencer is down based on the reasons given in 1). Furthermore, the given example for impact mentions USDC de-pegging while the sequencer is offline, but the on-chain value being used will not have de-pegged, meaning users would not be able to be liquidated for the duration that the sequencer is offline. Also, the issue mentions over-borrowing, but again users will not be able to borrow easily while the sequencer is offline AND all msg.sender addresses that initiate an L1-to-L2 transaction will have their address aliased, so users wouldn't even be able to submit orders because the msg.sender will not correspond to the address of their account on L2.

Additionally, this remark on the proposed recommendation will not do anything to protect users as soon as the sequencer is up and running again:

During the time when the sequencer is down, transactions should be reverted to prevent users from experiencing unexpected prices once the sequencer is up again. Therefore, I believe this issue should be classified as medium severity.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Trumpero]

Agree with the escalation. This issue should be a low/info

[hrishibhat]

Result:
Low
Has duplicates
Based on escalation comment 2., considering this issue a valid low as it clearly mentions the above said issues are not possible.

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• ArbitraryExecution: accepted