Missing bankruptcy detection when removing NFT collateral lets bad debt accumulate
Summary
When removing NFT collateral from an ERC721 pool, there's no detection whether the pool is bankrupt after the removal. As a result, the outstanding balance of LP tokens may be added to new deposits or moved to a different bucket. When added to new deposits, it may be redeemed for the newly deposited funds, causing a loss of funds for the depositors. When moved to a different bucket, it may be redeemed for that bucket's quote tokens, which will result in a loss of funds for the depositors of the bucket.
Vulnerability Detail
Pool can end up in a bankrupt state: a situation when there's no collateral or quote tokens in a pool, but there's still a positive number of LP tokens. Currently, the protocol detects pool bankruptcy in three cases:
If, as a result of removing liquidity from a pool via a call to ERC721Pool.removeCollateral, the pool ends up with 0 collateral, 0 quote tokens, and with a positive number of LP tokens, the bad debt won't be removed. The LP tokens can then be moved to a different bucket (using the moveQuoteToken function; the bankruptcy check will pass) and redeemed for quote tokens or collateral NFTs. Alternatively, the owner of the LPs may leave then in the bucket, wait for new collateral and/or quote tokens to be deposited by other lenders, and redeem the LPs for the new funds: the bankruptcy checks in LenderActions.removeQuoteToken, LenderActions.removeCollateral, and LenderActions._removeMaxCollateral will also pass.
Jeiwan
high
Missing bankruptcy detection when removing NFT collateral lets bad debt accumulate
Summary
When removing NFT collateral from an ERC721 pool, there's no detection whether the pool is bankrupt after the removal. As a result, the outstanding balance of LP tokens may be added to new deposits or moved to a different bucket. When added to new deposits, it may be redeemed for the newly deposited funds, causing a loss of funds for the depositors. When moved to a different bucket, it may be redeemed for that bucket's quote tokens, which will result in a loss of funds for the depositors of the bucket.
Vulnerability Detail
Pool can end up in a bankrupt state: a situation when there's no collateral or quote tokens in a pool, but there's still a positive number of LP tokens. Currently, the protocol detects pool bankruptcy in three cases:
However, there's no bankruptcy check when removing collateral via the ERC721Pool.removeCollateral function: the function calls LenderActions.removeCollateral, which doesn't have the bankruptcy check.
Impact
If, as a result of removing liquidity from a pool via a call to
ERC721Pool.removeCollateral, the pool ends up with 0 collateral, 0 quote tokens, and with a positive number of LP tokens, the bad debt won't be removed. The LP tokens can then be moved to a different bucket (using the moveQuoteToken function; the bankruptcy check will pass) and redeemed for quote tokens or collateral NFTs. Alternatively, the owner of the LPs may leave then in the bucket, wait for new collateral and/or quote tokens to be deposited by other lenders, and redeem the LPs for the new funds: the bankruptcy checks in LenderActions.removeQuoteToken, LenderActions.removeCollateral, and LenderActions._removeMaxCollateral will also pass.Code Snippet
LenderActions.sol#L379
Tool used
Manual Review
Recommendation
Consider adding the pool bankruptcy check to the LenderActions.removeCollateral function, similarly to how it's implemented in the LenderActions._removeMaxCollateral function.
Duplicate of #133