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

Full collateral liquidation for bid-offer takers settling partial points

0xbrivan2

Vulnerability Details

Bid-offer takers are required to deposit collateral equal to the amount they are selling to bid-offer makers, as demonstrated by the createTaker function. After the TGE, bid-offer takers must settle the points they sold to offer makers by calling settleBidTaker. However, if a taker settles only partial points, the current implementation results in the offer maker receiving both the settled points tokens and the entirety of the taker’s collateral:

function settleAskTaker(address _stock, uint256 _settledPoints) external {
// ...
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint *
_settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);
}
​
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
);
}
}

In the code above, the offer maker receives all the settled point tokens:

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

Additionally, if the taker does not settle all of their points, the offer maker also receives the entire collateral deposited by the taker:

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

This approach is unfair to the taker. Since the offer maker receives all the settled point tokens, they should only receive the portion of the collateral corresponding to the unsettled points. The taker should be refunded the collateral corresponding to the points they successfully settled.

Proof Of Concept

Consider the following example:

  1. As the Original Offer Maker, Alice posts a buy offer for 1,000 points with a unit price of $1 with 100% collateral parameter

  2. Bob accepts Alice's buy offer and sells 1,000 points to her, depositing $1,000 as collateral by calling createTaker

  3. The market owner updates the market and sets the TGE, initiating the settlement period.

  4. For whatever reason, Bob is only able to settle 500 points by calling settleAskTaker
    4.1 Alice receives all the settled point tokens. 500 of the 1,000 points are settled.
    4.2 Since Bob couldn't settle all his points, Alice incorrectly receives the entire $1,000 collateral deposited by Bob, instead of just the portion corresponding to the unsettled points.

Bob has settled 500 points but cannot withdraw the collateral corresponding to those settled points.

Impact

Bid-offer takers who settle only partial points will lose their full collateral, even though they have successfully settled part of their offer.

Tools Used

Manual Review

Recommendations

Consider refunding bid-offer makers only the portion of collateral corresponding to the unsettled points. Below is a suggested code update:

function settleAskTaker(address _stock, uint256 _settledPoints) external {
// ...
​
if (_settledPoints == stockInfo.points) {
// ...
​
}else {
+ uint256 refundAmount = Math.mulDiv(
+ stockInfo.points - _settledPoints,
+ offerInfo.amount,
+ offerInfo.points,
+ Math.Rounding.Ceil
+ );
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
offerInfo.authority,
makerInfo.tokenAddress,
- collateralFee
+ refundAmount
);
​
+ tokenManager.addTokenBalance(
+ TokenBalanceType.MakerRefund,
+ _msgSender(),
+ makerInfo.tokenAddress,
+ collateralFee - refundAmount
+ );
}
// ...
}
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.