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

In the `DeliveryPlace.closeBidTaker` function, collateral liquidation does not check whether the `preOffer` is aborted.

kupiasec

Github link

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

Summary

If the ask offer is settled, the bid taker of the offer receives the point tokens for settled point.
If the offer is aborted before settlement, the offer can't be settled after and the portion of collaterals for bidded points should be liquidated by bid taker.
And only if offer is aborted, collaterals should be liquidated.
But, even if offer is settled, it is available.
As a result, bid taker can receive unexpected collaterals and this causes the protocol's loss of funds.

Vulnerability Details

In the DeliveryPlace.closeBidTaker function, userCollateralFee is calculated using taker's userRemainingPoints from L179 and taker receives userCollateralFee amount of collateral.

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

uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
L179: offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);

But there is no check whether the preOffer is aborted in the function.
As a result, even when the preOffer is settled and not aborted, taker receive the collaterals.

Impact

The bid taker can receive unexpected collaterals and this causes the protocol's loss of funds.

Tools Used

Manual Review

Recommendations

It is recommended to change the code as following:

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

+ if (offerInfo.abortOfferStatus == AbortOfferStatus.Aborted){
uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);
+ }
Updates

Lead Judging Commences

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

finding-PreMarkets-closeBidTaker-lack-check-abort-status-drain

Valid high, for unsettled ask offers by the original maker, the initial remaining maker collateral is already refunded as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L624-L629)

Support

FAQs

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