Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Offer Status Not Changed from Virgin After Being Targeted in `createTaker` Leads to Several Issues

krisrenzo

Summary

The offer status is not updated from Virgin after being targeted in createTaker, allowing multiple takers to target the same offer address and leading to several issues, particularly where offerInfo.usedPoints is used.

Vulnerability Details

Tadle allows takers to target any offer they want to trade against when they call PreMarkets::createTaker. If the offer has a status of Virgin, it can be traded against.

if (offerInfo.offerStatus != OfferStatus.Virgin) {
revert InvalidOfferStatus();
}

The issue, however, is that the createTaker function does not update the corresponding offer status from Virgin, making it possible for multiple takers to target the same offers. This causes several issues within the protocol, particularly in areas where offerInfo.usedPoints is used for calculations.

Areas Where This Problem Manifests

- [UsedAmount calculated here would cause the user to receive less collateralFee than they are supposed to get](https://github.com/Cyfrin/2024-08-tadle/blob/72c93f73a26ec7472868cb509e8b454286810223/src/core/DeliveryPlace.sol#L162-L166) - [The protocol may incorrectly assume an offer status to be `Virgin` and conclude it has zero used points, leading to other accounting issues](https://github.com/Cyfrin/2024-08-tadle/blob/72c93f73a26ec7472868cb509e8b454286810223/src/core/DeliveryPlace.sol#L154-L157) - [Incorrect calculation of usedAmount](https://github.com/Cyfrin/2024-08-tadle/blob/72c93f73a26ec7472868cb509e8b454286810223/src/core/DeliveryPlace.sol#L286-L290) - [If another taker can transact with this offer maker, an issue arises as the usedPoints will increase, causing the refundAmount to increase and the protocol to overpay the caller](https://github.com/Cyfrin/2024-08-tadle/blob/72c93f73a26ec7472868cb509e8b454286810223/src/core/DeliveryPlace.sol#L66) // There are several cases of this in the code. A quick way to find them all is by grepping "usedPoints"

Impact

The accounting system becomes scrambled, causing both overestimation and underestimation of assets across all the contracts.

Tools Used

  • Manual

Recommendations

Only allow one taker to trade against an offer. Any other recommendations would be tedious as they would require rewriting the contracts to keep track of every One Maker to N Takers transaction.

Updates

Lead Judging Commences

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

krisrenzo Submitter
12 months ago
0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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