sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

5 stars 5 forks source link

0xShoonya - twap() unsafe on L2s in event of Sequencer downtime #110

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

0xShoonya

medium

twap() unsafe on L2s in event of Sequencer downtime

Summary

The twap() function in StrategyPassiveManagerVelodrome.sol can be exploited during Layer 2 (L2) sequencer downtime.

Vulnerability Detail

The twap() function uses the built in consult() function provided by Uniswap's Oracle Library to query the pool and determine the time weighted price. This takes in a secondsAgo and observes the price at secondsAgo and block.timestamp, returning the time weighted average between these two points.

In the event that we haven't had any observation since secondsAgo, we assume the latest observation still holds:

    function twap() public view returns (int56 twapTick) {
        uint32[] memory secondsAgo = new uint32[](2);
        secondsAgo[0] = uint32(twapInterval);
        secondsAgo[1] = 0;

        (int56[] memory tickCuml,) = IVeloPool(pool).observe(secondsAgo);
        twapTick = (tickCuml[1] - tickCuml[0]) / int32(twapInterval);
    }

In the event that an L2's sequencer goes down, the time weighted price when it comes back online will be the extrapolated previous price. This will create an opportunity to push through transactions at the old price before it is updated. Even when the new price is observed, it will be assumed by the sequencer that the previous price held up until the moment it came back online, which will result in a slow, time weighted adjustment back to the current price.

Note that, in the case of Arbitrum, there is the ability to force transactions through the delayed inbox. If other users are forcing transactions into the given pool, this could solve the problem, but if not it could also make the problem worse by allowing an attacker to force a transaction that abuses the outdated price while the sequencer is down, guaranteeing inclusion.

Impact

If the sequencer goes offline, the function uses outdated price data, allowing attackers to exploit the stale price before it updates. This vulnerability can lead to significant financial manipulation and arbitrage opportunities until the price is corrected.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L741-L748

Tool used

Manual Review

Recommendation

Use chainlink oracle instead of TWAP for L2s

sherlock-admin2 commented 2 months ago

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

z3s commented:

Invalid; From README: Can assume the sequencer will not misbehave.