Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`abortBidTaker` Devalues User Deposits

auditweiler

Summary

Invocations to abortBidTaker should refund the taker their collateral, however due to an unsafe sequence of operations, user deposits are vulnerable to severe precision loss.

Vulnerability Details

When aborting a taker's Bid for a maker's Ask, the taker is free to reclaim their capital via closeBidTaker (refunds via RemainingCash) or abortBidTaker (refunds via MakerRefund).

In the latter case, the calculated MakerRefund is computed as follows:

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

This is vulnerable to precision loss.

Let's consider the following offer.

CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000, /// @audit 1000_points
0.01 * 1e18, /// @audit 0.01e12 of USDC
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)

This combination of points and amount is used frequently (1 2 3) throughout PreMarkets.t.sol, and is therefore in the opinion of Tadle to be a rational order configuration):

If we imagine a taker opts to fulfill the entire offer (1_000 points), a refund of their input amount evaluates to as follows:

uint256 depositAmount = (1_000).mulDiv(
(1_000),
(0.01 * 1e18),
Math.Rounding.Floor
); /// === 0

The user's deposit is devalued to zero, since integer math evaluates:

(1000 * 1000) / 0.01e18
=== 1_000_000 / 10_000_000_000_000_000
=== 0

We should note that when applying the "real" precision of USDC (e6), the taker's deposit continues to be subject to excessive devaluation.

PreMarkets.t.sol

function test_abort_turbo_offer_devaluation() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
vm.stopPrank();
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
vm.startPrank(user);
// AbortAskOffer::MakerRefund 2000000000000000
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.stopPrank();
vm.startPrank(user1);
address stock1Addr = GenerateAddress.generateStockAddress(1);
/// @audit If we'd used `closeBidTaker`, we would be refunded as expected.
// CloseBidTaker::RemainingCash 12000000000000000
// deliveryPlace.closeBidTaker(stock1Addr);
​
/// @audit Instead, by using `abortBidTaker`, our deposit is lost completely.
// AbortBidTaker::MakerRefund 0
preMarktes.abortBidTaker(stock1Addr, offerAddr);
vm.stopPrank();
}

Consequently, the user's deposit has been lost using an order configuration especially supported by the team.

Impact

Excessive devaluation of claim to refunded collateral when using an intended (recommended) order configuration.

Tools Used

Manual Review

Recommendations

Calculate using a higher precision:

uint256 internal constant _PRECISION = 1e18;
​
uint256 depositAmount = (stockInfo.points * _PRECISION)
.mulDiv(preOfferInfo.points, preOfferInfo.amount, Math.Rounding.Floor) / _PRECISION;
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

[invalid] finding-Premarkets-depositAmount-precision-loss

Valid medium, given the free nature of the markets, it may be possible that such an offer type where both low amount of points correspond to lower decimal and lower amount of collateral value, so I believe medium severity is appropriate.

Appeal created

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

Support

FAQs

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