Adversary can block any `exit` due to `preCheck` reached `cap` by using flash-loan
Opened this issue · 6 comments
Github username: --
Twitter username: --
Submission hash (on-chain): 0xa152e0fc679f9840949b13a8713252bd1f4f032c33244d6bff807347e639bce3
Severity: medium
Description:
Description
There is a maximum dailycap
implemented on circuitBreaker contract to prevent any abnormal ovUSDC exits by users.
The preCheck
will increment current bucketIndex
amount, beside checking if the sum of rolling period buckets is still under the cap.
File: OrigamiCircuitBreakerAllUsersPerPeriod.sol
098: function preCheck(address /*onBehalfOf*/, uint256 amount) external override onlyProxy {
...
118: uint256 _newUtilisation = _currentUtilisation(_nBuckets) + amount;
119: if (_newUtilisation > cap) revert CapBreached(_newUtilisation, cap);
120:
121: // Unchecked is safe since we know the total new utilisation is under the cap.
122: unchecked {
123: // slither-disable-next-line weak-prng
124: buckets[_nextBucketIndex % _nBuckets] += amount;
125: }
126: }
The issue here is, attacker can flash-loan in order to fill-up the rolling period until it reached its cap.
By using flash-loaned USDC, then investWithToken
oUSDC and exitToToken
in a single transaction. This flash loan can trigger preCheck
and fill up the cap
easily.
File: OrigamiLendingSupplyManager.sol
146: function exitToToken(
147: address account,
148: IOrigamiInvestment.ExitQuoteData calldata quoteData,
149: address recipient
150: ) external override onlyOToken returns (
151: uint256 toTokenAmount,
152: uint256 toBurnAmount
153: ) {
...
157: // Ensure that this exit doesn't break the circuit breaker limits for oToken
158: circuitBreakerProxy.preCheck(
159: address(oToken),
160: account,
161: quoteData.investmentTokenAmount
162: );
Attack Scenario
- User get a flash-loaned asset USDC
- User buys oUSDC (
investWithToken
) with some amount of USDC from the flash-loan
OrigamiInvestmentVault::investWithToken()
->OrigamiOToken::investWithToken()
->OrigamiLendingSupplyManager::investWithToken()
->OrigamiLendingClerk::deposit
, there is no fee deducted here, thus USDC -> oUSDC will be 1:1, and get shares ovUSDC - User exit to USDC again, by calling
OrigamiLendingSupplyManager::exitToToken()
and passing all shares, to circuit breakerpreCheck
(and save it to current index bucket), then fill it near the cap - in the rolling period, the cap is now filled, any exits (or rebalance) will be reverted.
This issue can cause temporary DoS, preventing legitimate user exit ovUSDC normally due to filled cap.
Attachments
-
Proof of Concept (PoC) File
-
Revised Code File (Optional)
Recommendation
Consider implementing a preventive measure to disallow both invest and exit in the same block. This would introduce a delay or separation between these operations, reducing the risk of the flash-loan attack scenario.
To make it clear, the flow would be:
- Flash-loan USDC
- Check circuit breaker's
cap
andcurrentUtilisation()
(for oUSDC) - Get amount to fill up the gap between cap and currentUtilisation
investWithToken
, from USDC -> oUSDC,fromTokenAmount
= gap amountexitToToken
, from oUSDC -> USDC,investmentTokenAmount
= gap amount- this
exitToToken
in manager will triggerpreCheck
, updating bucket to cap, fill up utilisation.
Using these steps which is done in one transaction can freeze user exit due to daily cap reached
Thanks for the finding @chainNue
While there's no economic benefit, someone could do this as a DoS.
We'll consider if we remediate with:
- A cooldown as you suggest
- An economic deterrent (an exit fee)
- An improved circuit breaker implementation which can consider both deposits and exits (although i suspect this will be tricky, especially in a gas efficient manner)
Thanks @frontier159 for accepting this issue (and my other issue as well),
Indeed, there is no economic benefit, a short-term freezing of user funds is a medium one. If there is no mechanism to mitigate this issue, someone can easily block exit by filling up the cap, thus valid user hardly exit their asset, unless protocol intervene by increasing the cap, but that not preventing user doing the same attack again (or the cap lose its meaning)
Agree with the solution you're going to take, a cooldown and an exit fee I think is enough to mitigate this issue.
Agree with the solution you're going to take, a cooldown and an exit fee I think is enough to mitigate this issue.
To be clear i think either of those is enough, don't need both
yes, you are right, it's an OR not an AND
Another solution is to disable exits on the same block number as investments. Kind of a super short cooldown, only affecting invest-and-exit in the same transaction (flash loans).