Funding.citadelPriceInAsset can quickly become stale while its update can be front run

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L202-L207

Vulnerability details

Impact

It is possible to enter the Citadel sale at a price substantially different from the 1 USD equivalent targeted.

As WBTC, CVX markets periodically experience highly volatile moves an attacker will monitor for stale citadelPriceInAsset and enter on the big enough difference with current market, possibly front running an updateCitadelPriceInAsset() call.

Proof of Concept

CTDL to be received is calculated with the citadelPriceInAsset price as _assetAmountIn * citadelPriceInAsset:

https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L202-L207

citadelPriceInAsset updates that to be done on sharp moves of the market can be front run:

https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L412-L441

Recommended Mitigation Steps

Consider introducing additional ordinary spot oracle (say BTC/USD for WBTC) or liquid DEX pool (Uniswap V3 WBTC/USDC with $130m liquidity as of time of this writing; or even make two calls, for WBTC/ETH and ETH/USDC pools, both with $300m+) and prohibit the sale when Oracle/DEX price is too far from the citadelPriceInAsset.

This way the attack should be: wait for the recorded price to be off the marker and manipulate the pool in the same direction. This has too low expected value, so such an attack surface is much less attractive vs simply waiting for the recorded price to become outdated and buy.

In the same time the cost of reading DEX pool or spot price oracle is not prohibitive.

[jack-the-pug]

Downgrading to QA as this is based on an assumption of how the citadelPerAssetOracle works, which is out of scope.