Last minute supply transaction to avoid liquidation can be reverted

Summary

Each user gets an NFT minted to them to manage their positions. Each NFT is denoted by an Id, which is known as _nextId. The first user to mint an NFT will get the _nextId = 1, the next one minted will have 2 and so on. User's can supply, borrow, withdraw or repay through their token Ids.

All the functions i.e _supply(), _borrow(), _withdraw(), _repay() have a feature at the start of the function call which will accomadate params.tokenId if given as 0. This means that when performing any of the above function, A user can provide tokenId = 0 and if they are the last person to have minted an NFT which means if they are the owner of _nextId - 1 token, their params.tokenId will automatically be set to this.

if (params.tokenId == 0) {
      if (msg.sender != _ownerOf(_nextId - 1)) revert NFTErrorsLib.NotTokenIdOwner();
      params.tokenId = _nextId - 1;
    }

Vulnerability Detail

Consider the following scenerio.

Bob has minted himself an NFT through which they have supplied collateral to the protocol and taken a loan, but their collateral factor is not very high and just sufficient. To avoid being liquidated Bob keeps an eye on the mempool to note if any drastic price changes occur and they are in danger of liquidations so they can front run the liquidation tx and supply more collateral.

After a few hours, Bob's position becomes in danger and he wants to cover his loan with more collateral so they prepare the transaction. Bob considers that the code accomadates for the tokenId sent as 0 so he sents the transaction thinking they are the last person to have minted an NFT.

A malicious user at the same time is watching the mempool and notices bob's tx trying to frontrun the liquidation call. The malicious user frontruns both calls and mints a new NFT and increases the _nextId by 1, this means that bob's NFT is no longer the last minted NFT and due to the line

if (msg.sender != _ownerOf(_nextId - 1)) revert NFTErrorsLib.NotTokenIdOwner();

Bob's transaction will revert and cause the liquidation to go through. In DeFi, users are allowed to frontrun liquidation calls to avoid being liquidated and save their positions but in this case a malicious user has frontrun and reverted the user's transaction causing the liquidaton to occur.

Impact

Last Minute Supply to avoid Liquidations can be reverted by a griefer. Although the exact likelihood of this happening is very low, the impact is drastic. Since the code itself accomodates the user's transaction by filling their tokenId, this also does not fall onto bob as a User mistake.

Code Snippet

supply

if (params.tokenId == 0) {
      if (msg.sender != _ownerOf(_nextId - 1)) revert NFTErrorsLib.NotTokenIdOwner();
      params.tokenId = _nextId - 1;
}

Tool used

Manual Review

Recommendation

Do not accomadate the user's transaction by filling their tokenId, Since an issue came up with the exact code in the previous Audit, I would recommend removing this and not allowing 0 tokenId to be passed in.

Duplicate of #59

sherlock-audit / 2024-06-new-scope-judging

denzi_ - Last minute supply transaction to avoid liquidation can be reverted #505