Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Wrong Calculation of Amount deposited in closeBidTaker function. setting marker to TRUE instead of FALSE

bigsam

Summary

A critical error has been found in the `closeBidTaker` function where the bid taker is incorrectly classified as a maker during the calculation of the deposit amount. This mistake leads to the wrong amount being sent by the bid taker, potentially causing financial discrepancies.

Vulnerability Details

The vulnerability arises in the `closeBid` function, where the calculation of the deposit amount for the bid taker is incorrectly handled. Specifically, the code mistakenly sets the bid taker as a maker:

/**
* @notice Close bid taker
* @dev caller must be stock authority
* @dev stock type must Bid
* @dev offer status must be Settled
* @param _stock stock address
*/
function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
​
if (stockInfo.preOffer == address(0x0)) {
revert InvalidStock();
}
​
if (stockInfo.stockType == StockType.Ask) {
revert InvalidStockType();
}
​
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
​
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
,
​
) = getOfferInfo(stockInfo.preOffer);
​
OfferInfo memory offerInfo;
uint256 userRemainingPoints;
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {
offerInfo = perMarkets.getOfferInfo(makerInfo.originOffer);
if (stockInfo.offer == address(0x0)) {
userRemainingPoints = stockInfo.points;
} else {
OfferInfo memory listOfferInfo = perMarkets.getOfferInfo(
stockInfo.offer
);
userRemainingPoints =
listOfferInfo.points -
listOfferInfo.usedPoints;
}
}
​
if (userRemainingPoints == 0) {
revert InsufficientRemainingPoints();
}
​
if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
​
uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
@audit >> This should be false for a taker >> true,
Math.Rounding.Floor
);
}

In the `getDepositAmount` function, the `_isMaker` parameter is set to `true`, which is incorrect for a bid taker. According to the intended logic:

@audit >> READ>>> * @param _isMaker is maker, true if create offer, false if create offer
* @param _rounding rounding
*/
function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
@audit >> bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {

For taker orders, `_isMaker` should always be set to `false`. By setting it to `true`, the function incorrectly multiplies the amount by the collateral rate:

return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);

This leads to the wrong deposit amount being calculated and sent, as the amount should not be multiplied when the order is made by a taker.

Impact

This vulnerability results in bid takers sending the wrong deposit amount. Specifically, they end up sending an amount that has been incorrectly multiplied

Tools Used

- Manual code review

Recommendations

To fix this issue, it is essential to ensure that the `_isMaker` parameter is correctly set to `false` for bid takers. The logic should be adjusted as follows:

### Corrected Code Example

uint256 collateralFee;
​
if (offerInfo.usedPoints > offerInfo.settledPoints) {
​
if (offerInfo.offerStatus == OfferStatus.Virgin) {
​
collateralFee = OfferLibraries.getDepositAmount(
​
offerInfo.offerType,
​
offerInfo.collateralRate,
​
offerInfo.amount,
​
-- true,
++ false, // Correctly set to false for takers
​
Math.Rounding.Floor
​
);
​
}
​
}
​

This change ensures that the deposit amount for takers is calculated correctly, without applying the incorrect multiplication by the collateral rate.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-closeBidMaker-isMaker--false

Invalid, the computations are correct, when taker close a bid offer, of type `Bid` represented in their stock, the offerType of maker must be that of `Ask` as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L137-L139) when the offer is created. In which `(_offerType == OfferType.Ask && _isMaker) ` will result in the following computations performed as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/OfferLibraries.sol#L44-L51), so the collateral will be refunded appropriately,

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.