Open code423n4 opened 2 years ago
Valid issue. Good catch.
Severity should be lowered to 2 as it doesn't allow direct theft of funds and the loss would only occur under specific external conditions.
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements
https://docs.code4rena.com/roles/wardens/judging-criteria#estimating-risk-tl-dr
To my understanding, bad position would affect the whole protocol and a loss would have to be paid by other participans which means funds can be drained. For that reason, I'm keeping high risk.
Handle
WatchPug
Vulnerability details
When the liquidator is trying to liquidate a undercolldarezed loan by calling
liquidateAccount()
, it calls_unwrapUniPosition()
->uniV3Helper.removeLiquidity()
->positionManager.decreaseLiquidity()
.However, when the Uni v3 position has 0 liquidity,
positionManager.decreaseLiquidity()
will fail.See: https://github.com/Uniswap/v3-periphery/blob/main/contracts/NonfungiblePositionManager.sol#L265
Based on this, a malicious user can escaped liquidation by depositing a Uni v3 position with 0 liquidity.
Impact
Undercollateralized debts cannot be liquidated and it leads to bad debts to the protocol.
A malicious user can take advantage of this by creating long positions on the collateral assets and take profit on the way up, and keep taking more debt out of the protocol, while when the price goes down, the debt can not be liquidated and the risks of bad debt are paid by the protocol.
Proof of Concept
positionManager.decreaseLiquidity()
reverts.Recommendation
Check if liquidity > 0 when removeLiquidity.