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

Wrong amount of tokens will be returned to buyers when they call abortBidTaker due to miscalculation of depositAmount.

touthang

Summary

When a seller abort the offer, the buyer needs to call abortBidTaker in order to take back the amount he paid for the points.

Vulnerability Details

The amount to return to buyer is calculated by:

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
);
​
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);

As we can see the amount to transfer back is calculated using the depositAmount.

However, this depositAmount calculation is wrong since it does: points * totalPoints / amount when it should have been points * amount / totalPoints.

Lets see an example where;

  • Alice sells 2000 points for 1000 USDC with 150% collateral, therefore Alice sends 1500 USDC as collateral.

  • Bob buys 2000 points from Alice for 1000 USDC, Bob now has a stock with: stockInfo.points = 2000

  • Alice decides she doesn't want to sell the points anymore and calls abortAskOffer to abort the offer, Alice got back 500 USDC. 500 + 1000(from Bob) = 1500

  • Now, Bob calls abortBidTaker get back the amount he paid for those 2000 points i.e. 1000 USDC.

  • But with the current implementation of the calculation, Bob recieves a balance of 4000 USDC; 2000 * 2000 / 1000 = 4000.

  • The pool loses a net amount of 3000 USDC.

Impact

Depends on the points to amount ratio given by the seller, can either be:

  • Loss of funds for the protocol i.e. buyer recieves more, OR

  • Loss of funds for the buyer(if 1 point was worth more than 1 token) i.e. buyer will recieve amount lesser than what he paid.

Tools Used

manual

Recommendations

change the depositAmount calculation to:

uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
+ preOfferInfo.amount,
+ preOfferInfo.points,
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.