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

There are Premature Event Emissions in Several Functions in the `TokenDivider.sol` Contract which May Cause Potential State Discrepancies and Off-Chain Automation Risks

victortheoracle

Summary

In the TokenDivider contract, several events are emitted before the respective state changes or actions they represent have been finalized. This could lead to inconsistencies and confusion, especially when relying on these events for off-chain automation or analysis in the case that these functions are delayed or fail before completion.

Vulnerability Details

  1. divideNft Function:

    • The NftDivided event is emitted before the ERC20 token transfer is performed. This could result in the event being misleading if the transfer subsequently fails.

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L131-L136

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

  1. claimNft Function:

    • The NftClaimed event is emitted before the NFT is transferred from the contract to the caller's address. In the case that this fails, this event would be misleading.

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L164-L166

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

  1. transferErcTokens Function:

    • The TokensTransfered event is emitted before the actual ERC20 token transfer occurs. If the token transfer fails, the event will have already been emitted, leading to incorrect assumptions about the transfer's success.

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L205-L207

@> emit TokensTransfered(amount, tokenInfo.erc20Address);
​
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);

  1. sellErc20 Function:

    • The OrderPublished event is emitted before the order creation process is fully completed, which might not reflect the accurate state of the contract at the time the event is captured.

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L246-L248

@> emit OrderPublished(amount, msg.sender, nftPegged);
​
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, address(this), amount);

  1. buyOrder Function:

    • The OrderSelled event is emitted here before the transfer of ether in this function is completed. If the transfer goes wrong somewhere, the emitted event would be misleading.

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L288-L305

@> emit OrderSelled(msg.sender, order.price);
​
// Transfer The Ether
​
(bool success,) = payable(order.seller).call{value: (order.price - 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);

Impact

Emitting events prematurely can cause discrepancies between the actual contract state and the state gotten from the events. This is particularly problematic for:

  • Off-chain services and monitoring tools that rely on emitted events to trigger actions.

  • Users or developers debugging issues, as the event logs do not accurately represent the state changes.

Tools Used

  • Manual code review

Recommendations

To address this issue, move the code that handles the emitting of the events in all the mentioned functions to the end of those function after the states and/or transfer logic has been completed.

Updates

Lead Judging Commences

fishy Lead Judge 4 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.