Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

When Mode Is Protected, Protocol Assumes the Maker Hasn't Settled Their Offer and Allows the Taker Excess Withdrawal

krisrenzo

Summary

When the mode is protected, the protocol incorrectly assumes that the maker hasn’t settled their offer and allows the taker to withdraw the full amount of collateral tokens.

Vulnerability Details

When DeliveryPlace::closeBidTaker is called by a taker who has been settled by their maker in protected mode, the protocol assumes the maker hasn’t settled any of the taker's points. It sets the userRemainingPoints variable to the taker's stockInfo.points and allows the taker to withdraw all their collateral tokens.

uint256 userRemainingPoints;
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {

The function assumes that the offer hasn’t been settled yet. However, the maker may have settled some or all of the points transacted with their taker.

If the related offer's settledPointTokenAmount is greater than zero, which it will be if the maker has settled, the taker can withdraw their full collateral and some point tokens.

uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
​
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);
​
uint256 pointTokenAmount = offerInfo.settledPointTokenAmount.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress,
pointTokenAmount
);

Note that userCollateralFee is the deposit the taker made to buy points from the maker when they called PreMarkets::createTaker.

Impact

This issue can result in the loss of other users' funds, as the protocol may overpay the taker.

Tools Used

  • Manual

Recommendations

The issue originates from where userRemainingPoints is set. Instead of setting the variable to the taker's stockInfo.points, it should be set to the actual amount of points the maker has settled for the taker, which is the maker's offerInfo.settledPoints.

Updates

Lead Judging Commences

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

[invalid] finding-PreMarkets-closeBidTaker-userRemaining-points-wrong-set

Valid high, regardless for turbo or protected mode, partial settlements are possible as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L286-L299). For protected mode, partial settlements are not accounted for, allowing more then intended points to be sent to users even when maker only performed a partial settlement Note for invalidation: Agree with the discussions above that this issue is invalid.  Protected Mode is a step-by-step process. Example: > A created a purchase order for 800 points, B bought 300 points of it, and C bought 500 points of it. C re-listed 500 points, and D bought 200 points of them. > Settlement phase. A settles with B and C. C settles with D. A pays 800 point tokens, and C pays 200 point tokens to D, so the total Balance will have 1000 point tokens. Additionally, for any maker that does not settle, they will lose their original collateral posted in protected mode as it will force the admin to step in to settle.

Appeal created

0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

[invalid] finding-PreMarkets-closeBidTaker-userRemaining-points-wrong-set

Valid high, regardless for turbo or protected mode, partial settlements are possible as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L286-L299). For protected mode, partial settlements are not accounted for, allowing more then intended points to be sent to users even when maker only performed a partial settlement Note for invalidation: Agree with the discussions above that this issue is invalid.  Protected Mode is a step-by-step process. Example: > A created a purchase order for 800 points, B bought 300 points of it, and C bought 500 points of it. C re-listed 500 points, and D bought 200 points of them. > Settlement phase. A settles with B and C. C settles with D. A pays 800 point tokens, and C pays 200 point tokens to D, so the total Balance will have 1000 point tokens. Additionally, for any maker that does not settle, they will lose their original collateral posted in protected mode as it will force the admin to step in to settle.

Support

FAQs

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