Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Incorrect `collateralFee` Calculation Due to Inconsistent `_isMaker` Parameter

krisrenzo

Summary

The collateralFee returned when a Bid Taker is reimbursed is incorrect due to inconsistent use of the _isMaker parameter in the getDepositAmount function. This occurs when a Maker fails to settle all their points.

Vulnerability Details

When a Bid Taker is reimbursed for their collateral after a Maker fails to settle all their points, the collateralFee calculated by DeliveryPlace::closeBidTaker uses the same function that was used to calculate the required collateral when the Taker created their stock. Specifically, it uses the OfferLibraries::getDepositAmount function to determine the collateralFee, which is analogous to transferAmount in PreMarkets::_depositTokenWhenCreateTaker.

if (offerInfo.usedPoints > offerInfo.settledPoints) { // true when Maker fails to fully settle Taker
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
}

In PreMarket::_depositTokenWhenCreateTaker:

function _depositTokenWhenCreateTaker() internal {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Ceil
);
​
transferAmount = transferAmount + platformFee + tradeTax;
​
tokenManager.tillIn{value: msg.value}();
}

The issue arises because both instances of the function use different boolean values for the _isMaker parameter of [OfferLibraries::getDepositAmount]. The implementation in DeliveryPlace::closeBidTaker is incorrect, as the caller in this context is the Taker, not the Maker, and is attempting to get a refund for their unsettled points.

function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {
/// @dev bid offer
if (_offerType == OfferType.Bid && _isMaker) {
return _amount;
}
​
/// @dev ask order
if (_offerType == OfferType.Ask && !_isMaker) { // Returned when BidTaker calls createTaker
return _amount;
}
​
return Math.mulDiv( // Returned when BidTaker calls closeBidTaker
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);
}

Inspection of the OfferLibraries::getDepositAmount logic reveals that this oversight has significant implications, as the return value differs greatly depending on the case.

Impact

The protocol reimburses the Bid Taker an incorrect amount when they call for a refund of their unsettled points, potentially leading to financial discrepancies.

Tools Used

Manual analysis.

Recommendations

Use false instead of true when calling OfferLibraries::getDepositAmount() in DeliveryPlace::closeBidTaker.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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,

Appeal created

krisrenzo Submitter
about 1 year ago
krisrenzo Submitter
about 1 year ago
krisrenzo Submitter
about 1 year ago
krisrenzo Submitter
about 1 year ago
krisrenzo Submitter
about 1 year ago
0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year 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.