Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Inconsistent Handling of Partial Token Fetch and Maker Refund in Settlement Process

cryptomoon

Summary

The settleAskMaker function contains a critical inconsistency in how it handles the partial fetching of tokens from the maker and the logic surrounding maker refunds. Specifically, the function fetches partial tokens from the maker even when the maker is not eligible for a refund unless all used points are settled. This inconsistency can lead to scenarios where the maker loses collateral without receiving a corresponding refund. Due to this, maker wouldn't settle any points if they can't settle all points because he wouldn't be getting his collateral back unless he settles all points.

Vulnerability Details

The function fetches partial tokens from the maker using the tokenManager.tillIn method, even when the maker is not settling all used points. However, the maker will not receive a refund unless all used points are settled. This inconsistency suggests that the maker might lose tokens without any possibility of a refund, which is unfair and could lead to unexpected financial losses for the maker.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L265-L307

ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
// FETCHES THE PARTIAL SETTLED POINTS TOKEN FROM MAKER
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
}
​
uint256 makerRefundAmount;
if (_settledPoints == offerInfo.usedPoints) {
// ONLY REFUNDS IF MAKER HAS SETTLED ALL THE POINTS
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = 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
);
​
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
​
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
}

Impact

Loss of funds of makerif they are trying to settle partial points.

Tools Used

Manual review

Recommendations

The logic needs to be clarified and aligned to ensure that if the maker is not settling all used points and is not eligible for a refund, no partial tokens should be fetched from them. Alternatively, if partial settlement is allowed, then a corresponding partial refund should be calculated and provided to the maker.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-settleAskMaker-partial-settlements

Valid high, in settleAskTaker/settleAskMaker, if the original offer maker performs a partial final settlement, the existing checks [here](https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L356-L358) and [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L230-L232) will cause an revert when attempting to complete a full settlement, resulting in their collateral being locked and requiring a rescue from the admin. To note, although examples in the documentation implies settlement in a single click, it is not stated that partial settlements are not allowed, so I believe it is a valid user flow.

Support

FAQs

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