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

The `DeliveryPlace.settleAskTaker` function manages the collateral incorrectly.

kupiasec

Github link

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L395

Summary

If _settledPoints < stockInfo.points, the DeliveryPlace.settleAskTaker function transfers collaterals for stockInfo.amount, not for settledPointTokenAmount to offerInfo.authority.
But, offerInfo.authority should receive the liquidated collaterals for unsettled points.
And collaterals for settledPointTokenAmount should be refunded to stockInfo.authority.

Vulnerability Details

In the DeliveryPlace.settleAskTaker function, if _settledPoints < stockInfo.points, collaterals for stockInfo.amount are transferred to offerInfo.authority from L395 and L410.

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L392-L414

uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
L395: stockInfo.amount,
false,
Math.Rounding.Floor
);
if (_settledPoints == stockInfo.points) {
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
collateralFee
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
L410: offerInfo.authority,
makerInfo.tokenAddress,
collateralFee
);
}

But, collaterals for unsettled points should be liquidated to offerInfo.authority and collaterals for settledPointTokenAmount should be transferred to stockInfo.authority.

Impact

  • stockInfo.authority who provided the collateral can not receive collaterals for settledPointTokenAmount and this causes the stockInfo.authority's loss of funds.

  • offerInfo.authority receives more collaterals than actual liquidation amount.

Tools Used

Manual Review

Recommendations

In the DeliveryPlace.settleAskTaker function, if _settledPoints < stockInfo.points, transfer the collaterals for unsettled points to offerInfo.authority.
And transfer collaterals for settledPointTokenAmount to stockInfo.authority.

Updates

Lead Judging Commences

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