Pieces Protocol

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

Reentrancy Risk in divideNft Function

Summary

The divideNft function in line 129 exhibits a potential reentrancy vulnerability. The sequence of operations within the function is as follows:

  1. Internal State Modification: The contract updates its internal state by modifying the balances mapping to reflect the user's new token balance.

  2. Token Transfer: The contract attempts to transfer tokens to the user using IERC20(erc20).transfer(msg.sender, amount).

  3. the bug arises because The function transfers tokens to the user after modifying internal state

Vulnerability Details

This order of operations creates an opportunity for a malicious erc20Contract. If the transfer() function within erc20Contract is designed to be reentrant, it could:

  • Re-enter divideNft: The malicious erc20Contract could, upon receiving the initial transfer, call back into the divideNft function before the initial call completes.

  • Exploit State: The divideNft function would then proceed to modify the internal state again, potentially leading to unintended consequences, such as:

    • Incorrect balance updates

    • Unauthorized token transfers

    • Loss of funds

Impact

A successful reentrancy attack could result in:

  • Financial Loss: The contract could lose funds due to incorrect accounting or unauthorized transfers.

  • System Instability: The contract's state could become inconsistent, potentially leading to unexpected behavior or malfunctions.

  • Reputation Damage: A successful exploit could damage the reputation of the project and erode user trust.

Tools Used

Recommendations

Check-Effects-Interactions Pattern:

  • Check the balance of the contract before the transfer.

  • Perform the transfer.

  • Check the balance of the contract after the transfer to ensure the expected amount was transferred.

  • Only then modify the internal state.


bool private _notEntered;

modifier nonReentrant() {````require(_notEntered, "ReentrancyGuard: reentrant call");````_notEntered = true;````_;````_notEntered = false;````}

function divideNft(...) nonReentrant public {````// ... rest of the code ...````}

Updates

Lead Judging Commences

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

Support

FAQs

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