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

Incorrect Collateral Distribution in Partial Settlements Leads to Economic Imbalance and Potential Exploitation

cheatcode

Summary

The settleAskTaker function is not properly handling the case where _settledPoints is less than stockInfo.points. Specifically:

  1. When _settledPoints < stockInfo.points, the function calculates collateralFee based on the full stockInfo.amount, which is incorrect.

  2. In this case, it also refunds the entire collateralFee to the offer authority, which is not proportional to the settled points.

Code

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

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
);
}

The function should calculate the collateralFee proportionally to the _settledPoints and distribute it accordingly.

Impact

Overpayment of collateral could drain the contract of funds more quickly than intended, potentially leading to liquidity problems if this occurs frequently or with large amounts.

The current implementation might incentivize partial settlements, as the offer authority would receive the full collateral refund even for partial settlements. This could lead to gaming of the system, where users might prefer partial settlements to maximize their returns unfairly.

Mitigation

  1. Calculate the collateralFee proportionally to _settledPoints.

  2. Distribute the collateralFee between the caller and the offer authority based on the ratio of settled to unsettled points.

Poc

function test_incorrect_collateral_distribution_in_partial_settlement() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
​
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskTaker(stock1Addr, 250);
vm.stopPrank();
​
uint256 offerAuthorityBalance = tokenManager.userTokenBalanceMap(
offerAddr,
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
​
uint256 expectedRefund = (0.01 * 1e18 * 500) / 1000; // Full collateral for 500 points
assertEq(offerAuthorityBalance, expectedRefund, "Incorrect collateral refund to offer authority");
​
uint256 callerBalance = tokenManager.userTokenBalanceMap(
user,
address(mockUSDCToken),
TokenBalanceType.RemainingCash
);
assertEq(callerBalance, 0, "Caller incorrectly received collateral");
​
uint256 totalDistributedCollateral = offerAuthorityBalance + callerBalance;
assertEq(totalDistributedCollateral, expectedRefund, "Total distributed collateral is incorrect");
}
Updates

Lead Judging Commences

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

finding-PreMarkets-partial-closeBidTaker-userCollateralFee-wrong-partial

Valid High, afaik, partial settlements are a valid flow and so when closing bid offers by takers and/or when settling offers by makers, we should return a proportionate amount of funds based on points settled. This issues could be related to issue #1008, but seems to be describing a different issue.

Appeal created

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