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

PreMarkets - Invalid formula in `abortBidTaker` function

Hrom131

Summary

Incorrect formula for calculating depositAmount value inside abortBidTaker function of PreMarkets contract may result in loss of user's funds.

Vulnerability Details

Inside the abortBidTaker function of the PreMarkets contract there is an error in the formula for calculating the depositAmount value.

Code fragment of the abortBidTaker function where the mistake occurred:

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

The mistake in the formula is that preOfferInfo.points and preOfferInfo.amount should be swapped around.

Consider the following case as an example:

  1. User creates the offer to sell 1000 points for 10 ERC20 tokens. This token has 18 decimals.

  2. Next, some User2 decides to buy 250 points. To do this, he calls createTaker and pays 2.5 ERC20 tokens.

  3. Next, the User who created the original offer decides to abort it. To do this, he calls the abortAskOffer function.

  4. Then User2 decides to abort the buy offer and calls abortBidTaker function with the hope of getting his 2.5 ERC20 tokens back, but due to a mistake in the formula the user will get back 0 tokens and a Finished buy offer. That is, he will lose his money.

The user will get 0 because the formula will have the following calculations:

  • depositAmount = stockInfo.points * preOfferInfo.points / preOfferInfo.amount

  • depositAmount = 250 * 1000 / (10 * 10^18) = 0

What should be the calculations in the correct formula:

  • depositAmount = stockInfo.points * preOfferInfo.amount / preOfferInfo.points

  • depositAmount = 250 * (10 * 10^18) / 1000 = 2500000000000000000 (2.5 ERC20 tokens)

Test to illustrate the existing vulnerability

To run the test, its code should be placed in the PreMarkets.t.sol file.

function test_invalid_abort_bid_taker_deposit_amount_formula() public {
vm.startPrank(user);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
10 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
address makerAddr = GenerateAddress.generateMakerAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
​
vm.startPrank(user1);
​
uint256 pointsToBuy = 250;
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, pointsToBuy);
​
MakerInfo memory makerInfo = preMarktes.getMakerInfo(makerAddr);
OfferInfo memory offerInfo = preMarktes.getOfferInfo(offerAddr);
​
address user1StockAddr = GenerateAddress.generateStockAddress(1);
​
StockInfo memory stockInfo = preMarktes.getStockInfo(user1StockAddr);
​
uint256 expectedDepositAmount = Math.mulDiv(
stockInfo.points,
offerInfo.amount,
offerInfo.points,
Math.Rounding.Floor
);
uint256 expectedTransferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
expectedDepositAmount,
false,
Math.Rounding.Floor
);
​
console.log("Expected depositAmount - ", expectedDepositAmount);
console.log("Expected transferAmount - ", expectedTransferAmount);
​
vm.stopPrank();
​
vm.prank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
vm.prank(user1);
preMarktes.abortBidTaker(user1StockAddr, offerAddr);
​
stockInfo = preMarktes.getStockInfo(user1StockAddr);
​
console.log("User1 MakerRefund amount - ", tokenManager.userTokenBalanceMap(user1, address(mockUSDCToken), TokenBalanceType.MakerRefund));
console.log("Stock status - ", uint8(stockInfo.stockStatus)); // Finished
}

Impact

Due to an error in the formula refund amount will be incorrectly calculated, which will result in loss of user funds.

Tools Used

The vulnerability was discovered during a manual audit of the protocol's contract code. To test the vulnerability for validity and demonstrate it, a unit test was written.

Recommendations

The formula simply needs to be corrected.

Here's the correct code:

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

Lead Judging Commences

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