Tadle

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

incorrect collateralAmount handling when maker does not fully settle Points

Summary

in the settleAskTaker when the _settledPoints is not equal to the points of the stockInfo for the askTaker, it enters the else statement, which incorectly takes the whole collateral of the taker and gives it to the bidMaker which is the offer Authority, without taking into consideration how much points the taker actually settled. And the taker still transfers the settled points to the offerAuthority

Vulnerability Details

settleAskTaker

if (_settledPoints == stockInfo.points) {
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
collateralFee
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
offerInfo.authority,
makerInfo.tokenAddress,
collateralFee
);
}

When it comes to know how much of the points you are actually settling if it is not directly equal to the point of the stock you hold it takes the whole collateral, this would be correct if the function did not still transfer the calculated settlement ten line before this snippet

if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);
}

So we have a situation that when askTakers settle, they pay the settlement amount and if the points are not equal to the stock point they get the whole collateral taken and added to the makers TokenBalanceType.MakerRefund

Impact

The point of collateral is to assign it to the amount of points promised to be delivered by the askTaker, if the collateral gets removed then this causes loss of funds to the taker as they would loos the collateral even though they settled part of the points

Tools Used

manual review

Recommendations

The ratio of the amount of collateral to be removed should be equal to the ratio of settledPoints to the Stockinfo.points, which will accuratley describe how much collateral to add to the offer authority balance and how much to add to the takers balance.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.

Give us feedback!