H-04 MitigationConfirmed

[c4-bot-7]

Lines of code

Vulnerability details

C4 issue

H-04: V3Utils.execute() does not have caller validation, leading to stolen NFT positions from users

Comments

When interacting with V3Utils.execute(), EOA users will first need to approve their NFT before calling execute(). This leads to EOA executing two transactions. Unfortunately, V3Utils.execute() does not validate the execute() caller that they are the NFT token owner. This allows a malicious user to create a sandwich between the victim's transactions and use the victim NFT for malicious purpose.

The core of this issue lies in the fact that execute() does not have any validations against the msg.sender.

Mitigation

PR #29

This PR makes several changes for multiple issues. For our purposes with this ticket, we are focused on some of the changes:

• A Transformer contract is created. This contract contains validation logic that V3Utils can use to validate the msg.sender.
• V3Utils inherits from the Transformer contract. This allows V3Utils.execute() to utilize the _validateCaller function.
• The Transformer._validateCaller() contains several safety checks. This safety checks include:
  • Checks if the msg.sender is the tokenId owner. This check is necessary if a EOA user is interacting with V3Utils.execute().
  • Checks if the owner is owned by the V3Utils contract. This check is necessary when a token is transferred to V3Utils. When a NFT token is transferred to V3Utils, V3Utils.onERC721Received() will be called which then calls execute(). In this scenario, V3Utils owns the NFT and calling execute() -> _validateCaller() will not revert.

Based on the above changes, users are no longer able to utilize a token they do not own within V3Utils.execute() for malicious purposes.

Conclusion

LGTM

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 marked the issue as confirmed for report