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

When a User Aborts his/her BidTakerOrder the user will receive a wrong amount due to an error in the Deposit Amount Calculation.

bigsam

Summary

A critical issue has been identified in the `AbortBidTaker` function, where an incorrect deposit amount calculation results in users receiving the wrong amount of funds when they abort their order. This miscalculation could lead to users losing funds or receiving an incorrect refund.

Vulnerability Details

The vulnerability arises from an error in the formula used to calculate the deposit amount when returning funds to the user. The current implementation uses the following formula:

/**
* @notice abort bid taker
* @param _stock stock address
* @param _offer offer address
* @notice Only offer owner can abort bid taker
* @dev Only offer abort status is aborted can be aborted
* @dev Update stock authority refund amount
*/
function abortBidTaker(address _stock, address _offer) external {
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage preOfferInfo = offerInfoMap[_offer];
​
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
​
if (stockInfo.preOffer != _offer) {
revert InvalidOfferAccount(stockInfo.preOffer, _offer);
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus(
StockStatus.Initialized,
stockInfo.stockStatus
);
}
​
if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}
​
@audit>> Wrong Application >>> uint256 depositAmount = stockInfo.points.mulDiv(
@audit>> Wrong Application >>> preOfferInfo.points,
@audit>> Wrong Application >>> 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
);
​
stockInfo.stockStatus = StockStatus.Finished;
​
emit AbortBidTaker(_offer, _msgSender());
}
​

This formula incorrectly calculates the deposit amount as `(stockInfo.points * preOfferInfo.points) / preOfferInfo.amount`.

This leads to the user receiving an incorrect refund amount when they abort their order. The correct calculation should instead be based on obtaining the fraction of the amount to be added back to the taker’s balance, similar to the formula used when the user initially opened the order:

/**
* @notice Create taker
* @param _offer offer address
* @param _points points
*/
function createTaker(address _offer, uint256 _points) external payable {
-------------------------------------
​
/// @dev Transfer token from user to capital pool as collateral
@audit>> Correct formula when user Opened the taker order >>>> uint256 depositAmount = _points.mulDiv(
@audit>> Correct formula when user Opened the taker order >>>> offerInfo.amount,
@audit>> Correct formula when user Opened the taker order >>>> offerInfo.points,
Math.Rounding.Ceil
);

```

The discrepancy between these two formulas leads to errors in the deposit amount calculation, impacting the accuracy of the refund sent back to the user.

Impact

This vulnerability could result in users receiving an incorrect amount of funds when they abort their `BidTakerOrder`.

Tools Used

- Manual code review

Recommendations

To resolve this issue, the deposit amount formula should be corrected to ensure accurate calculations when funds are returned to the user. The recommended formula is as follows:

/**
* @notice abort bid taker
* @param _stock stock address
* @param _offer offer address
* @notice Only offer owner can abort bid taker
* @dev Only offer abort status is aborted can be aborted
* @dev Update stock authority refund amount
*/
function abortBidTaker(address _stock, address _offer) external {
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage preOfferInfo = offerInfoMap[_offer];
​
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
​
if (stockInfo.preOffer != _offer) {
revert InvalidOfferAccount(stockInfo.preOffer, _offer);
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus(
StockStatus.Initialized,
stockInfo.stockStatus
);
}
​
if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}
​
-- uint256 depositAmount = stockInfo.points.mulDiv(
-- preOfferInfo.points,
-- preOfferInfo.amount,
-- Math.Rounding.Floor
-- );
​
++ uint256 depositAmount = stockInfo.points.mulDiv(
​
++ preOfferInfo.amount,
​
++ preOfferInfo.points,
​
++ Math.Rounding.Floor
​
);
​

This correction will ensure that the correct fraction of the amount is added back to the taker’s balance, thereby preventing any discrepancies in the amount returned to the user.

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.