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

Incorrect `depositAmount` calculation in `PreMarkets::abortBidTaker()`

0xnbvc

Vulnerability Details

In the PreMarkets contract, when a taker aborts their position, they should receive their initial deposit back. However, the abortBidTaker() function contains an incorrect calculation for the deposit amount, which results in the taker receiving nothing upon aborting.

The deposit amount is incorrectly calculated as follows in the abortBidTaker() function:

PreMarkets.sol#L671-L675

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

This results in a deposit amount calculated as:

depositAmount = stockInfo.points * preOfferInfo.points / preOfferInfo.amount (rounding down)

This calculation is inconsistent with the correct formula used in the createTaker() function, which properly calculates the deposit amount:

PreMarkets.sol#L211-L216

/// @dev Transfer token from user to capital pool as collateral
uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);

Correct calculation:

depositAmount = _points * offerInfo.amount / offerInfo.points (rounding up)

Impact

Loss of funds for the users.

Proof of Concept

  1. Alice creates an offer with CreateOfferParams.points = 1000 and CreateOfferParams.amount = 0.01 * 1e18 (10_000_000_000_000_000).

  2. Bob calls createTaker() with _points = 500 and has to deposit 0.005 USDC:

    1. depositAmount = 500 * 10_000_000_000_000_000 / 1_000 = 5_000_000_000_000_000 / 1e18 = 0.005

  3. Alice aborts the offer with abortAskOffer() to get back her deposit.

  4. Bob aborts their position with abortBidTaker() to get back his deposit, but the calculation is incorrect:

    1. depositAmount = 500 * 1_000 / 10_000_000_000_000_000 = 0.00000000005 / 1e18 = 0.00000000000000000000000000005 = 0 (rounded down by Solidity)

Recommendations

Replace the incorrect calculation in PreMarkets::abortBidTaker() with the correct formula:

uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
preOfferInfo.amount,
+ preOfferInfo.points,
- Math.Rounding.Floor
+ Math.Rounding.Ceil
);
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.