OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

Reentrancy risk in amendSellOrder() could lead to lost funds

ariiellus

Root + Impact

Description

In the amendSellOrder() function, the contract calls token.safeTransfer(...) to return the difference in token amount to the msg.sender before updating the internal state of the order.

While this is currently safe due to the limited set of supported tokens, it introduces a potential reentrancy vector when in the future the protocol allows arbitrary or unverified tokens (e.g., memecoins) to be traded.

This violates the Checks-Effects-Interactions pattern, which is crucial to prevent reentrancy.

@>https://github.com/CodeHawks-Contests/2025-07-orderbook/blob/fdef247b2f2bd7c0f9c19310406c8e072d9ffda4/src/OrderBook.sol#L166

Risk

Likelihood:

  • When the protocol evolves and allows the trading of not verified tokens

Impact:

  • Lost of funds from the protocol

Proof of Concept

Although this is not currently exploitable, it becomes critical if new tokens are allowed in the future or dynamic listing is implemented.

An hipothetical scenario for an attack will be:
​
1. Deploying a malicious token contract with a fallback that reenters the amendSellOrder() function.
​
2. Call createSellOrder() to create an order using the malicious token.
​
3. Call amendSellOrder() to trigger the refund logic (safeTransfer) before the state is updated.
​
4. In the fallback of the token, reenter the function to steal funds.

Recommended Mitigation

Consider adding a validation check before user token.safeTransfer

- uint256 diff = order.amountToSell - _newAmountToSell;
- token.safeTransfer(order.seller, diff);
- }
+ uint256 diff = order.amountToSell - _newAmountToSell;
+ order.amountToSell = _newAmountToSell;
+ token.safeTransfer(order.seller, diff);
+ }
Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 1 month ago
yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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