Invalid Offer Status Check in createTaker Function
Relevant links
Summary
The createTaker function incorrectly validates the offer status by ensuring that only offers with a Virgin status can be processed. This logic potentially prevents ongoing offers from filling taker orders, which contradicts the intended functionality of the protocol.
Vulnerability Details
The createTaker function contains a condition that checks if the offerStatus of a given offer is Virgin. If the status is not Virgin, the function reverts with an InvalidOfferStatus error.
According to the intended functionality of the protocol, even ongoing offers (those that have moved past the Virgin status but still have remaining points or quantity) should be able to fill taker orders as long as they have sufficient quantity. The current logic prevents this, which could lead to unintended disruptions in the offer-taker matching process.
Impact
The impact of this vulnerability is significant as it directly affects the core functionality of the protocol. By restricting ongoing offers from filling taker orders, users may be unable to execute valid transactions, leading to decreased liquidity and user dissatisfaction.
PoC
Create an offer and move it to an ongoing state (any state other than Virgin).
Attempt to create a taker order for the ongoing offer using the createTaker function.
Observe that the transaction reverts with an InvalidOfferStatus error, even though the offer should be valid for filling taker orders.
Tools Used
Manual review
Recommendations
Modify the conditional check in the createTaker function to allow ongoing offers to fill taker orders. The logic should ensure that the offer has sufficient points or quantity to fulfill the taker order rather than solely relying on the Virgin status.
Lead Judging Commences
finding-PreMarkets-createTaker-Ongoing-Status
The reason for severity for this issue and duplicates are very similar to issue #1164. However, in this case, the issues correctly identified that offer statuses should be updated accordingly based on points transaction (partially filled orders = `Ongoing`, fully filled orders = `Filled`). There is currently no impact on the tadle system since the above two statuses are unused, and while sementically `Virgin` status is not the correct status representing whether taker orders can be created agains maker offers, it can still be performed without a DoS. However, if we are basing it off of the correct status implementation (i.e. `Ongoing` phase appropriately updated when takers create taker offers), then the DoS will occur, essentially blocking any taker offers from being created by subsequent takers for partially filled orders. All issues that does not mention the DoS impact will be low severity, since they still correctly highlighted the wrong status accounting. All issues that mention the possible bypass of `Virgin` status is incorrect, because the usedPoint checks will always ensure points are filled only up to the points posted for offer as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L180-L186). Note for downgrade to low severity: Agree with appeals and low severity, this is more of a status accounting error and does not have any impact, since the statuses consistently do not utilize a switch from Vigin to Ongoing/Filled and the protocol can function appropriately even without the use of such statuses (presuming other bugs are fixed), the DoS impact will not occur.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.