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

Incorrect refund amount calculation in `abortBidTaker`

0xbrivan2

Vulnerability Details

When ask makers decide to abort their offers, takers who have already accepted the offer can use the abortBidTaker function to claim a refund of the amount they deposited. However, there is an issue with the function incorrectly calculating the deposited amount for the takers:

function abortBidTaker(address _stock, address _offer) external {
// ...
uint256 depositAmount = stockInfo.points.mulDiv(
>>> preOfferInfo.points,
>>> preOfferInfo.amount,
Math.Rounding.Floor
);
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
}

The deposited amount should be calculated as points in the stock * the value of each point. However, the function is incorrectly calculates the value of each point as follows:

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points, // @audit this should be in the denominator
preOfferInfo.amount, // @audit this should be in the numerator
Math.Rounding.Floor
);

The current calculation is (stock points * offer points) / offer amount, which is incorrect. The correct calculation should be (stock points * offer amount) / offer points. The current incorrect calculation will result in a refund of 0 due to the representation of the offer amount with 18 decimals (amount in the denominator >> points * offer.points in the numerator).

Proof Of Concept

The following test demonstrates the issue. To reproduce, copy and paste the test into test/PreMarkets.t.sol:

function test_wrongRefundCalculationOnAbortBidTaker() public {
vm.startPrank(user1);
capitalPool.approve(address(mockUSDCToken));
​
// Alice (user) create an Ask offer of 1,000 points with amount $1,000
vm.startPrank(user);
uint points = 1000;
uint amount = 1000 * 1e18;
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
points,
amount,
10000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
// Bob (user2) takes the offer and buys 500 points from Alice
vm.startPrank(user2);
preMarktes.createTaker(offerAddr, 500);
​
// Alice aborts her ask offer
vm.startPrank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
// Bob calls `abortBidTaker` to get back the amount deposited for buying from Alice
vm.startPrank(user2);
address user2StockAddr = GenerateAddress.generateStockAddress(1);
vm.expectEmit(true, true, true, true);
// Notice last param (0) which refers to the refund amount to be sent to Bob
emit ITokenManager.AddTokenBalance(address(user2), address(mockUSDCToken), TokenBalanceType.MakerRefund, 0);
​
preMarktes.abortBidTaker(user2StockAddr, offerAddr);
uint user2BalanceBeforeWithdrawing = mockUSDCToken.balanceOf(address(user2));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
uint user2BalanceAfterWithdrawing = mockUSDCToken.balanceOf(address(user2));
​
// Bob did not receive his deposit, and his stock now is FINISHED
assert(user2BalanceAfterWithdrawing == user2BalanceBeforeWithdrawing);
}
forge test --mt test_wrongRefundCalculationOnAbortBidTaker
​
[PASS] test_wrongRefundCalculationOnAbortBidTaker() (gas: 941980)

Impact

Ask takers will receive a refund of 0 if the pre-offer is aborted due to the incorrect refund amount calculation.

Tools Used

Manual Review

Recommendations

Update the calculation of the deposit amount as follows:

// function:abortBidTaker
​
function abortBidTaker(address _stock, address _offer) external {
// ..
​
uint256 depositAmount = stockInfo.points.mulDiv(
+ preOfferInfo.amount,
+ preOfferInfo.points
- preOfferInfo.points,
- preOfferInfo.amount,
Math.Rounding.Floor
);
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
// ...
}
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.