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

Wrong event emission in multiple functions in the Tokendivider.sol contract

udogodwin2k22

Summary

The contract emits events before executing critical state changes and transfers, which can lead to incorrect event logs in cases where the subsequent operations fail. This creates a discrepancy between what actually happened on-chain and what the events indicate happened, potentially misleading users and applications that rely on these events for transactions.

Vulnerability Details

In several functions throughout the contract, events are emitted before the actual state-changing operations or transfers take place. Here are the specific instances:

In the claimNft() function:

emit NftClaimed(nftAddress);
// NFT transfer happens after the event
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);

In the divideNft() function:

emit NftDivided(nftAddress, amount, erc20);
// Token transfer happens after the event
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if(!transferSuccess) {
revert TokenDivider__TransferFailed();
}

In the buyOrder() function:

emit OrderSelled(msg.sender, order.price);
// ETH transfer happens after the event
(bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
if(!success) {
revert TokenDivider__TransferFailed();
}

If an event is emitted before a transfer, and the transfer fails, the contract’s state will not be updated to reflect the failed transaction. This can lead to discrepancies between the expected state and the actual state of the contract.

Impact

  1. Incorrect Blockchain Data: If a transfer fails after an event is emitted, the event log will indicate that an action succeeded when it actually failed. This creates an incorrect record of contract interactions.

  2. Integration Issues: Third-party applications, users, or protocols that rely on these events for tracking state changes might malfunction or make incorrect decisions based on the misleading event data.

Recommendation

Events should be emitted after all state changes and transfers have been successfully completed. Here's how the code should be modified:

For the claimNft() function:

IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);
emit NftClaimed(nftAddress);

For the divideNft() function:

bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if(!transferSuccess) {
revert TokenDivider__TransferFailed();
}
emit NftDivided(nftAddress, amount, erc20);

For the buyOrder() function:

(bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
if(!success) {
revert TokenDivider__TransferFailed();
}
emit OrderSelled(msg.sender, order.price);
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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