The original code for V3Utisl.execute does not check if the caller is the owner of the NFT, this allows anyone to call execute on any tokenId that has been approved to V3Utils.
Mitigation
PR #29
The mitigation code added check for V3Utisl.execute function:
function execute(uint256 tokenId, Instructions memory instructions) public returns (uint256 newTokenId) {
_validateCaller(nonfungiblePositionManager, tokenId);
}
function _validateCaller(INonfungiblePositionManager nonfungiblePositionManager, uint256 tokenId) internal view {
if (vaults[msg.sender]) {
uint256 transformedTokenId = IVault(msg.sender).transformedTokenId();
if (tokenId != transformedTokenId) {
revert Unauthorized();
}
} else {
address owner = nonfungiblePositionManager.ownerOf(tokenId);
if (owner != msg.sender && owner != address(this)) {
revert Unauthorized();
}
}
}
It's now can only be called from the vault, the V3Utils contract itself or the owner of the tokenId.
The mitigation solved the original issue.
Lines of code
Vulnerability details
C4 issue
H-04: V3Utils.execute() does not have caller validation, leading to stolen NFT positions from users
Comment
The original code for
V3Utisl.execute
does not check if the caller is the owner of the NFT, this allows anyone to callexecute
on anytokenId
that has been approved to V3Utils.Mitigation
PR #29 The mitigation code added check for
V3Utisl.execute
function:It's now can only be called from the vault, the V3Utils contract itself or the owner of the tokenId. The mitigation solved the original issue.
Conclusion
LGTM