Supply part of the accrued debt can be stolen

[code423n4]

Handle

cmichel

Vulnerability details

The LendingPair.uniClaimDeposit function allows the user to "collect fees" and mint new supply shares with the collected amounts.

uniClaimDeposit does not accrue tokens

However, the current total supply is not accrued in the function. This means an attacker can:

• mint shares using uniClaimDeposit
• increase the totalSupplyAmount by calling accrue(token0) and accrue(token1) afterwards.
• call withdraw and receive a larger amount of tokens for the newly minted shares due to the increase in totalSupplyAmount from accrue (increasing the supply share price _sharesToSupply).

This would only lead to a small protocol loss if uniClaimDeposit would only collect the fees, however, combined with another flaw, one can steal almost the entire protocol lpRate each time:

uniClaimDeposit allows collecting entire liquidity instead of just fees

This has to do with the way liquidity from a Uniswap V3 position (NFT) is withdrawn:

• when calling positionManager.decreaseLiquidity, the position.liquidity is removed but stored in the position as tokensOwed0/tokensOwed1. It is not transferred to the user
• one needs to call positionManager.collect(params) to actually transfer out these tokens, setting tokensOwed0/1 to 0. (This is correctly done in UniswapV3Helper.removeLiquidity.)

An attacker can perform the following attack:

• Create a Uniswap V3 position.
• Get flashloans for both tokens to provide lots of liquidity for this position.
• Call positionManager.decreaseLiquidity such that the entire liquidity is removed and stored (but not collected yet) in the position's tokensOwed0/1 fields
• Deposit it to WildCredit's lending pair using depositUniPosition
• Call uniClaimDeposit to mint a huge amount of NFT supply shares. This huge amount will capture the protocol's debt accrual in the next steps.
• Call accrue on both tokens to accrue debt and pay the lpRate part of it to suppliers, increasing totalSupplyAmount and thus the value of a supply share.
• With the new debt added to the totalSupplyAmount, the attacker can now withdraw their minted shares again and capture most of the new debt that was accrued, making a profit.

Impact

Combining these two issues, an attacker could steal most of the accrued lpRate in a single atomic transaction. The attacker can repeat this step capturing the supplier interest for each accrual. (The longer the market hasn't been accrued, the bigger the profit per single attack transaction, but in the end, the attacker could perform this attack at every block or when it becomes profitable for the gas costs.)

Providing / removing Uniswap V3 liquidity does not incur fees.

The attacker's profit is the loss of other legitimate suppliers that capture less of the newly accrued debt.

Recommendation

Accrue the debt for both tokens first in LendingPair.uniClaimDeposit.

It might also be a good idea to disallow collecting the "parked" liquidity in a token (that has been removed but not yet collected) by immediately collecting them when the NFT is deposited in depositUniPosition. I.e., call _uniCollectFees in depositUniPosition to withdraw any outstanding tokens&fees. Then mint shares with these token amounts.

[talegift]

We'll implement the suggested fix.

Suggest lowering severity to 2 as it doesn't allow direct theft of funds and the loss would only occur under specific external conditions - long periods of not accrue interest combined with a low gas price to steal the pending interest.

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

[ghoul-sol]

It seems that the attacker can steal interest that is owed to other users but deposits are safe. For that reason I agree with sponsor to make this medium risk.