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

Reentrancy Attack in buyOrder() function

ramprakash

Context: https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L261-L307

Summary:

The buyOrder()function in the Tokendivider.sol smart contract is vulnerable to a reentrancy attack. An attacker can exploit the function's external calls (Ether and ERC20 token transfers) to re-enter the function and manipulate its behavior, potentially draining funds or disrupting the contract's logic.

Vulnerability Details :

The BuyOrder function performs external calls before updating the contract's state. This allows an attacker to re-enter the function and perform malicious actions before the contract updates its state.

Attack can take place like:

  • A malicious contract places a sell order.

  • The attacker calls buyOrder and triggers a reentrant call during the Ether transfer to their address.

  • The contract’s state (e.g., balances or sell orders) has not yet been updated, enabling the attacker to manipulate the same or other sell orders.

Impact :


Funds Theft: The attacker can repeatedly trigger reentrant calls to drain Ether or ERC20 tokens by exploiting the lack of state updates before external calls.

Tools Used :

  • Manual Code Review: Analyzed the order of operations and identified the improper placement of state updates after external calls.

  • Foundry : Useful for fuzz testing and validating reentrancy scenarios.

Recommendations :

  • "Adopt the Check-Effects-Interactions Pattern" to ensure all state updates occur before any external interactions.

  • "Use OpenZeppelin's ReentrancyGuard" to add the nonReentrant modifier to the buyOrder() function to prevent reentrant calls.

  • "Safe Ether Transfers" to use low-level call instead of transfer or send for Ether transfers and handle reentrancy risks explicitly.

Solution:

  1. Integrate OpenZeppelin's ReentrancyGuard:

    • This prevents reentrant calls by ensuring a single execution at a time.

    • Add the nonReentrant modifier to the buyOrder() function.

  2. Update State Before External Calls:

    • Move balance updates and sell order modifications before making external calls.

    • This ensures the contract is in a safe state before any potential reentrant attack.

  3. Perform External Calls After Updates:

    • Ether transfers callare performed after all state updates.

Fixed Code (buyOrder()function) :

// Necessary Imports
​
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
​
// Add nonReentrant modifier at
// line 21 contract TokenDivider is IERC721Receiver, Ownable, ReentrancyGuard
contract TokenDivider is IERC721Receiver, Ownable, ReentrancyGuard {
// Contract body
}
​
// Add nonReentrant modifier at buyOrder()function
function buyOrder(uint256 orderIndex, address seller) external payable nonReentrant {
if (seller == address(0)) {
revert TokenDivider__InvalidSeller();
}
​
SellOrder memory order = s_userToSellOrders[seller][orderIndex];
​
if (msg.value < order.price) {
revert TokenDivider__IncorrectEtherAmount();
}
​
uint256 fee = order.price / 100; // Platform fee (1%)
​
//error corrected from
// uint256 sellerFee = fee / 2;
//to
uint256 sellerFee = order.price - fee;
​
if (msg.value < order.price + fee) {
revert TokenDivider__InsuficientEtherForFees();
}
​
balances[msg.sender][order.erc20Address] += order.amount;
​
s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1];
s_userToSellOrders[seller].pop();
​
emit OrderSelled(msg.sender, order.price);
​
(bool success, ) = payable(order.seller).call{value: sellerFee}("");
if (!success) {
revert TokenDivider__TransferFailed();
}
​
(bool taxSuccess, ) = payable(owner()).call{value: fee}("");
if (!taxSuccess) {
revert TokenDivider__TransferFailed();
}
​
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}
​
​
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Reentrancy

Appeal created

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.