Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Reentrancy Vulnerability in claimNft Function

sauronsol

Description:
The claimNft function in the TokenDivider contract is vulnerable to reentrancy attacks. The function burns ERC20 tokens and transfers the NFT to the caller without following the Checks-Effects-Interactions pattern. An attacker can exploit this by implementing a malicious contract that re-enters the claimNft function during the NFT transfer.

Impact:

  • An attacker could potentially claim the same NFT multiple times while only burning the ERC20 tokens once

  • This could lead to a loss of funds for the protocol and its users

  • The contract's state could become inconsistent

Proof of Concept:

// File: test/unit/ClaimNftReentrancyTest.t.sol
contract ClaimNftReentrancyTest is Test {
TokenDivider public tokenDivider;
ERC721Mock public nft;
address constant ATTACKER = address(1);
uint256 constant TOKEN_ID = 1;
uint256 constant AMOUNT = 1e18;
​
function setUp() public {
nft = new ERC721Mock();
tokenDivider = new TokenDivider();
// Mint NFT and approve TokenDivider
nft.mint(address(this), TOKEN_ID);
nft.approve(address(tokenDivider), TOKEN_ID);
// Divide NFT into ERC20 tokens
tokenDivider.divideNft(address(nft), TOKEN_ID, AMOUNT);
// Get ERC20 token address
Vm.Log[] memory logs = vm.getRecordedLogs();
address erc20Address;
for (uint i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == keccak256("NftDivided(address,address)")) {
erc20Address = address(uint160(uint256(logs[i].topics[2])));
break;
}
}
// Transfer tokens to attacker
vm.startPrank(address(this));
tokenDivider.transferErcTokens(erc20Address, ATTACKER, AMOUNT);
vm.stopPrank();
}
​
function testReentrancyInClaimNft() public {
vm.startPrank(ATTACKER);
// Enable reentrancy in NFT contract
nft.setReentrancy(true, address(tokenDivider));
// Claim NFT
tokenDivider.claimNft(address(nft));
vm.stopPrank();
// Verify that the NFT belongs to the attacker
assertEq(nft.ownerOf(TOKEN_ID), ATTACKER, "NFT should belong to attacker");
}
}

Tools Used:

  • Manual code review

  • Foundry for testing

  • Slither static analysis

Recommendations:

  1. Implement the Checks-Effects-Interactions pattern:

function claimNft(address nftAddress) external {
// Checks
if (nftAddress == address(0)) revert TokenDivider__InvalidAddress();
address erc20Address = nftToErc20[nftAddress];
if (erc20Address == address(0)) revert TokenDivider__InvalidAddress();
// Effects
IERC20(erc20Address).burnFrom(msg.sender, AMOUNT_OF_TOKENS_TO_MINT);
emit NftClaimed(nftAddress);
// Interactions
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, nftToTokenId[nftAddress]);
}

  1. Consider using OpenZeppelin's ReentrancyGuard:

contract TokenDivider is ReentrancyGuard {
function claimNft(address nftAddress) external nonReentrant {
// ... existing code ...
}
}
Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.