search

code-423n4 / 2024-03-revert-lend-findings

13 stars 10 forks source link

Anyone can use tokenId in execute #77

Closed c4-bot-1 closed 7 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/transformers/V3Utils.sol#L115-L352

Vulnerability details

Impact

In V3Utils::execute, users can Execute instruction by pulling approved NFT instead of direct safeTransferFrom call from owner. However there is no validation whether the caller is the owner of the tokenId or approved address.

Proof of Concept

As we can see in executeWithPermit, there is a validation:

        if (nonfungiblePositionManager.ownerOf(tokenId) != msg.sender) {
            revert Unauthorized();
        }

But there is no such check in execute or a check if the msg.sender is approved.

Consider the following scenario:

  1. User A approves nonfungiblePositionManager.
  2. User A calls execute with his instructions.
  3. User B sees the tx and frontruns User A, but with different instructions.

Tools Used

Manual Review

Recommended Mitigation Steps

Add validation if the msg.sender is owner or approved.

Assessed type

Access Control

c4-pre-sort commented 7 months ago

0xEVom marked the issue as duplicate of #141

c4-pre-sort commented 7 months ago

0xEVom marked the issue as sufficient quality report

c4-judge commented 7 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 7 months ago

jhsagd76 changed the severity to 3 (High Risk)