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

Buyer is never refounded if the seller aborts the offer

0xlrivo

Summary

Buyer is never refounded if the seller aborts the offer

Vulnerability Details

Suppose we have the following scenario:

  • user sells 1000 points for 10 USDC in proteced mode

    • user transfers 10 USDC as collateral since he's a seller

  • user1 buys all the points from user

    • user is credited with the eachTradeTax as TaxIncome

    • user is credited with 10 USDC as SalesRevenue, which are the 10 USDC spent by user1 to buy the points

  • user aborts his offer by calling abortAskOffer

  • user1 aborts his offer by calling abortBidTaker

In abortBidTaker the deposit amount of the buyer is computed as follows:

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

Especially with USDC (since it has 6 decimals) it can easily round down to 0, for example:

stockPoints = 1000
preOfferPoints = 1000
preOfferAmount = 1e7 (10 USDC)
​
depositAmount = (1000*1000) / 1e7 = 0.1 = 0 (since Solidity rounds down)

Impact

The buyer, user1 in this case, is never refounded of the amount he originally spent to buy the tokens.

The seller, user in this case, is never refounded of his collateral but he gets the USDC spent by the buyer even if the trade never settled.

POC

Add the following test to PreMarkets.t.sol:

function test_sell_offer_protected_chain_correctness() public {
vm.label(user, "seller"); // sells 1000 points
vm.label(user1, "buyer"); // buys 1000 points from user
​
capitalPool.approve(address(mockUSDCToken));
​
vm.startPrank(user); // USER SELLS 1000 POINTS FOR 10 USDC
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
10e6,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
); // -> offer0Addr e stock0Addr
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
vm.startPrank(user1); // USER1 BUYS 1000 POINTS FROM USER
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.createTaker(offerAddr, 1000); // -> stock1Addr (user is accredited the USDC spent by user1)
​
vm.startPrank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
vm.startPrank(user1);
preMarktes.abortBidTaker(stock1Addr, offerAddr);
​
vm.startPrank(user);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.TaxIncome); // eachTrade tax
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue); // user1 buy order
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund); // 0
​
vm.startPrank(user1);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund); // <@
}

In this scenario, when all the points are bought, nor the seller nor the buyer receive back their collateral.

Tools Used

  • Manual review

  • Foundry

Recommendations

The SalesRevenue should be accredited to the seller only after settlement.

The calculation of the depositAmount should account for the correct decimals of the token being used by the offer.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 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

0xbrivan2 Auditor
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.