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

Inconsistent Handling of Collateral Fee Distribution in settleAskTaker Function

cryptomoon

Summary

The settleAskTaker function contains a inconsistency in how it handles the distribution of collateral fees based on whether all points are settled. Specifically, the function transfers the full collateral fee to the taker if all points are settled, but if not all points are settled, the function transfers the full collateral fee to the maker. This approach leads to an inconsistency where the maker receives the full collateral fee even when only a portion of the points are unsettled,

Vulnerability Details

The settleAskTaker is designed to transfer the full collateral fee to the maker when not all points are settled. However, this is problematic because the maker should only receive the portion of the collateral fee corresponding to the unsettled points. Transferring the full collateral fee to the maker, even when only some points remain unsettled, results in loss to taker.

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

ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
​
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);
}
​
uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
stockInfo.amount,
false,
Math.Rounding.Floor
);
​
if (_settledPoints == stockInfo.points) {
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
collateralFee
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
offerInfo.authority,
makerInfo.tokenAddress,
collateralFee
);
}

Impact

Allowing the full collateral fee to be transferred to the maker when only a portion of the points are unsettled can lead to losses for the taker and unfair benefit to maker.

Tools Used

Manual review

Recommendations

The collateral fee should be proportionally distributed based on the number of points settled versus the total points. The taker should receive the collateral fee corresponding to the points they have settled, while the maker should only receive the collateral fee corresponding to the unsettled points.

Updates

Lead Judging Commences

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

finding-PreMarkets-partial-closeBidTaker-userCollateralFee-wrong-partial

Valid High, afaik, partial settlements are a valid flow and so when closing bid offers by takers and/or when settling offers by makers, we should return a proportionate amount of funds based on points settled. This issues could be related to issue #1008, but seems to be describing a different issue.

Appeal created

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.