Summary
When an Ask offer is partially settled, the maker (seller) does not receive the expected proportional refund of their collateral. This discrepancy could lead to significant financial losses for users and undermine the fairness of the protocol.
Vulnerability Details
[Relevant code] (https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L222-L325)
The vulnerability lies in the settleAskMaker function of the DeliveryPlace contract. When a maker partially settles an Ask offer, the function fails to correctly calculate and distribute the proportional refund of the collateral. Instead of receiving a partial refund corresponding to the settled portion, the maker receives no refund at all.
This issue arises from an incorrect implementation of the settlement logic, where the collateral refund calculation does not account for partial settlements. The function appears to be treating all settlements as full settlements, resulting in no refund for partial settlements.
POC
In existing test suite, add the following test
function testPartialSettlementDiscrepancy() public {
vm.startPrank(user);
deal(address(mockUSDCToken), user, 100000000 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1000 ether,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
vm.stopPrank();
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user2);
deal(address(mockUSDCToken), user2, 100000000 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
1 ether,
block.timestamp - 1,
3600
);
uint256 initialMakerBalance = tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.MakerRefund);
console2.log("Initial Maker Balance:", initialMakerBalance);
vm.startPrank(user);
deal(address(mockPointToken), user, 100000000 * 10 ** 18);
mockPointToken.approve(address(tokenManager), type(uint256).max);
OfferInfo memory offerInfoBefore = preMarktes.getOfferInfo(offerAddr);
console2.log("Offer Status Before:", uint(offerInfoBefore.offerStatus));
console2.log("Offer Used Points Before:", offerInfoBefore.usedPoints);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
uint256 finalMakerBalance = tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.MakerRefund);
console2.log("Final Maker Balance:", finalMakerBalance);
uint256 refundedAmount = finalMakerBalance - initialMakerBalance;
console2.log("Refunded Amount:", refundedAmount);
OfferInfo memory offerInfoAfter = preMarktes.getOfferInfo(offerAddr);
console2.log("Offer Status After:", uint(offerInfoAfter.offerStatus));
console2.log("Offer Used Points After:", offerInfoAfter.usedPoints);
console2.log("Offer Settled Points:", offerInfoAfter.settledPoints);
assertEq(refundedAmount, 500 ether, "Maker should receive half of the collateral for 50% settlement");
}
Impact
Loss of funds for makers in case of partial settlements
Tools Used
Manual Review, foundry
Recommendations
Add logic to fix the partial settlement correctly.
function settleAskMaker(address _offer, uint256 _settledPoints) external {
// ... (existing validation code)
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint * _settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn{value: msg.value}(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
}
- uint256 collateralFee;
- if (offerInfo.usedPoints > offerInfo.settledPoints) {
- if (offerInfo.offerStatus == OfferStatus.Virgin) {
- collateralFee = OfferLibraries.getDepositAmount(
- offerInfo.offerType,
- offerInfo.collateralRate,
- offerInfo.amount,
- true,
- Math.Rounding.Floor
- );
- } else {
- uint256 usedAmount = offerInfo.amount.mulDiv(
- offerInfo.usedPoints,
- offerInfo.points,
- Math.Rounding.Floor
- );
-
- collateralFee = OfferLibraries.getDepositAmount(
- offerInfo.offerType,
- offerInfo.collateralRate,
- usedAmount,
- true,
- Math.Rounding.Floor
- );
- }
- }
+ // Calculate collateral fee for the settled portion
+ uint256 settledAmount = offerInfo.amount.mulDiv(
+ _settledPoints,
+ offerInfo.points,
+ Math.Rounding.Floor
+ );
+ uint256 collateralFee = OfferLibraries.getDepositAmount(
+ offerInfo.offerType,
+ offerInfo.collateralRate,
+ settledAmount,
+ true,
+ Math.Rounding.Floor
+ );
+ // Calculate and distribute refund
+ uint256 refundAmount = collateralFee.mulDiv(
+ _settledPoints,
+ offerInfo.points,
+ Math.Rounding.Floor
+ );
+ tokenManager.addTokenBalance(
+ TokenBalanceType.MakerRefund,
+ offerInfo.authority,
+ makerInfo.tokenAddress,
+ refundAmount
+ );
+ // Calculate and distribute revenue
+ uint256 revenueAmount = settledAmount;
+ tokenManager.addTokenBalance(
+ TokenBalanceType.SalesRevenue,
+ offerInfo.authority,
+ makerInfo.tokenAddress,
+ revenueAmount
+ );
+ // Calculate and distribute tax
+ uint256 taxAmount = revenueAmount.mulDiv(
+ makerInfo.eachTradeTax,
+ 10000,
+ Math.Rounding.Floor
+ );
+ tokenManager.addTokenBalance(
+ TokenBalanceType.TaxIncome,
+ offerInfo.authority,
+ makerInfo.tokenAddress,
+ taxAmount
+ );
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
perMarkets.settledAskOffer(
_offer,
_settledPoints,
settledPointTokenAmount
);
+ emit SettleAskMaker(
+ makerInfo.marketPlace,
+ offerInfo.maker,
+ _offer,
+ _msgSender(),
+ _settledPoints,
+ settledPointTokenAmount,
+ refundAmount,
+ revenueAmount,
+ taxAmount
+ );
}
