Tadle

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

In the function `DeliveryPlace::settleAskTaker`, when the `Taker` delivered partial settlement, he still lost all of his collaterals instead of proportionally

Summary

When the Taker settled the points partially, the Maker of the Bid Offer can grab the entire amount of the Taker's collateral, instead of a prorated portion of it. However, the Taker has paid the portion of settle tokens, and the Maker can claim the refund for the unsettled portion accordingly, so the logic is unfair if the Maker can still get the entire amount of the Taker's collateral in this case.

Vulnerability Details

The venerable code are these lines, quoted below:

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
);
}

From the above code we can see that, the Maker can get the whole collateral rather than a proportional of it, even if the Taker may have delivered a partial settlement.

Impact

The directly impact of this issue is that, the Maker got extra money at the Taker's loss for every of such trade if the Taker only settles partially. This jeopardize the whole integrity of the trade, and it should be considered a H severity issue.

Tools Used

Manual Review

Recommendations

Calculate a proportional amount of the collateral to compensate the Maker in this case, instead of using the whole amount.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 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.