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

Users can drain the pool by aborting a BID order

dadekuma

Summary

When a bid order is aborted, the user will receive the wrong amount of collateral as a refund (higher or lower than expected depending on the offer condition).

Vulnerability Details

The calculation for getting back collateral after the order is aborted is wrong.
Currently, it's (stockInfo.points * preOfferInfo.points) / preOfferInfo.amount, but it should be instead (stockInfo.points * preOfferInfo.amount) / preOfferInfo.points:

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points,
preOfferInfo.amount,
Math.Rounding.Floor
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L671-L675

  1. Alice creates an ASK offer for 1000 points and 2000 collateral

  2. Bob creates a BID order to buy 400 points, sending 800 as payment

  3. Bob aborts and calls abortBidTaker, he should receive 800:

(stockInfo.points * preOfferInfo.amount) / preOfferInfo.points -> 400 * 2000 / 1000 = 800

But he received 200 instead, so he lost 600:

(stockInfo.points * preOfferInfo.points) / preOfferInfo.amount -> 400 * 1000 / 2000 = 200

  1. If offer points were worth less than the collateral (e.g. 2000 points for 1000 collateral), Bob could abuse this to drain the pool instead

Impact

Impact: High (Protocol funds drained or loss of user funds)
Likelihood: High (No preconditions)

Risk: Critical

Tools Used

Manual review

Recommendations

uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
+ preOfferInfo.amount,
- preOfferInfo.amount,
+ preOfferInfo.points,
Math.Rounding.Floor
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L671-L675

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-abortBidTaker-amount-wrong-StockInfo-points

Valid high severity, due to incorrect computation of `depositAmount` within `abortBidTaker`, when aborting bid offers created by takers, the collateral refund will be completely wrong for the taker, and depending on the difference between the value of `points` and `amount`, it can possibly even round down to zero, causing definite loss of funds. If not, if points were worth less than the collateral, this could instead be used to drain the CapitalPool contract instead.

Support

FAQs

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