Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

A malicious attacker can drain the protocol.

kupiasec

Github link

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L673

Summary

In the PreMarkets.abortBidTaker function, msg.sender receives tokens that he deposited in PreMarkets.createTaker function.
But, calculation of depositAmount is not correct and a malicious attacker can drain the protocol by leveraging this.

Vulnerability Details

In the createTaker function, depositAmount is calculated as Ceil(_points * offerInfo.amount / offerInfo.points)

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L212

depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);

In the abortBidTaker function, depositAmount is calculated as Floor(_points * offerInfo.points / offerInfo.amount)

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L673

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

depositAmount in createTaker and abortBidTaker function are different.

Let's assume the following scenario:

  • step1: Alice, the initial market maker, lists 1000e18 points for sale at 10 USDC(1e7 wei).

  • step2: Alice buys 500e18 points from origin offer of step1. She deposits 5 USDC.
    depositAmount = 500e18 * 1e7 / 1000e18 = 5e6

  • step3: She aborts bid taker of step2. She receives 5e28 USDC.
    depositAmount = Floor(500e18 * 1000e18 / 1e7) = 5e34

In step2, Alice deposits 5USDC, but she receive any 5e28 USDC(5e34 wei) in step3.
As a result, a malicious attacker can drain the protocol.

Impact

As a result, a malicious attacker can drain the protocol.

Tools Used

Manual Review

Recommendations

It is recommended to change the code as following:

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L673

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

Lead Judging Commences

0xnevi Lead Judge 10 months 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.