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

In case settled points is less than used points for a partially filled ask offer, a portion of the maker's collateral will stuck in the protocol

fyamf

Summary

When an ask offer is partially filled with a bid order, and then it is settled with amount of points less than used amount of points, the maker's collateral will be locked. Then when the taker closes his bid, he gets a portion of the maker's collateral. So, some part of the maker's collateral will stuck in the protocol and no one is able to withdraw them.

Vulnerability Details

  • Suppose Alice (maker) creates an ask offer to buy 1000 points for $1000 with collateral rate 12000 (equivalent to 120%). So, she deposits $1200.

  • Bob (taker) places a bid order to buy 700 points from Alice.

  • Then the market updates and TGE happens where tokenPerPoint = 1e18. So, it is expected that Alice provides 700 * 1e18 marketPlaceInfo.tokenAddress to keep her promise.

  • Suppose Alice has only 500e18 marketPlaceInfo.tokenAddress.

  • Then, Alice or the protocol owner can call the function settleAskMaker to settle this trade.

  • Alice is reasonable to not call this function, because she owns only 500 points while she promised 700 points, so her collateral will be locked in the protocol and she can not withdraw it during the settlement. Moreover, if she calls this function she has to provide some marketPlaceInfo.tokenAddress as well.

  • So, the owner will call this function to settle this trade (as Alice did not settle it). Please note that whether Alice or owner calls this function does not change the validity of this report.

  • Thus, the body of the following if-clause will not be executed during settlement (_settledPoints < usedPoints):

uint256 makerRefundAmount;
if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = 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
);
​
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
​
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
}

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L276

  • Then Bob calls the function closeBidTaker. Since offerInfo.usedPoints > offerInfo.settledPoints (as you remember usedPoints = 700 and settledPoints is 0 up to 500 based on whether settleAsMaker is called by the owner or Alice). So, the body of the following if-clause will be executed:

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

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L152

  • Since the offer status is settled, we will have:

usedAmount = $1000 * 700 / 1000 = $700
collateralFee = $700 * 12000 / 10000 = $840
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L162-L174

  • Then we will have $840 as the amount of Alice's collateral that will be paid to Bob as RemainingCash.

userCollateralFee = $840 * 700 / 700 = $840
uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
​
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);

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

  • The issue is that Alice deposited $1200 while the amount of collateral dedicated to Bob is only $840, it means that 1200 - 840 = $360 is stuck in the protocol and unreachable. Moreover, Alice can not withdraw them as her offer is settled.

  • Note that Bob places a bid order by depositing $700, and he is receiving $840.

  • Note that as explained above Alice will not call the function settleAsMaker because she does not want to lose marketPlaceInfo.tokenAddress. So, there will be no marketPlaceInfo.tokenAddress for Bob as well.

  • Please note that this bug is not at all related to the owner.

PoC

The following test implements the exact scenario explained above.

function test_stuck_collateral() public {
address Alice = vm.addr(10); // maker
address Bob = vm.addr(11); // taker
​
////// Alice(maker) creates a ask offer
vm.startPrank(Alice);
deal(address(mockUSDCToken), Alice, 1200 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1000 * 1e18,
12000,
0,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
vm.stopPrank();
////////////////
​
///// Bob (taker) creates a bid order
vm.startPrank(Bob);
deal(address(mockUSDCToken), Bob, type(uint256).max);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 700);
​
vm.stopPrank();
/////////////
​
///// owner updates the market and the TGE happens
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
1 * 1e18,
block.timestamp - 1,
3600
);
​
skip(3600); // time passes so we are not in MarketPlaceStatus.AskSettling anymore
​
vm.stopPrank();
//////////////
​
////// owner settles
// even if Alice settles the same issue happens
​
vm.startPrank(user1);
​
deliveryPlace.settleAskMaker(offerAddr, 0);
​
vm.stopPrank();
/////////////
​
//////// Bob closes
vm.startPrank(Bob);
address stock1Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock1Addr);
vm.stopPrank();
/////////////
​
console.log(
"Bob balance of USDC as RemainingCash: ",
tokenManager.userTokenBalanceMap(
address(Bob),
address(mockUSDCToken),
TokenBalanceType.RemainingCash
)
);
}

The output is:

Logs:
Bob balance of USDC as RemainingCash: 840000000000000000000

Impact

  • Stuck of collateral in the protocol without being able to withdraw them.

Tools Used

Recommendations

Maker should be able to withdraw part of her collateral that is not used even after the settlement.

Updates

Lead Judging Commences

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

finding-TokenManager-FOT-Rebasing

Valid medium, there are disruptions to the ability to take market actions. The following functions will be disrupted without the possibiliy of reaching settlement, since the respective offers cannot be created/listed regardless of mode when transferring collateral token required to the CapitalPool contract or when refunding token from user to capital pool during relisting. So withdrawal is not an issue - `createOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96-L102) - `listOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L355-L362) - `relistOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515-L521) - `createTaker()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831-L836) I believe medium severity is appropriate although the likelihood is high and impact is medium (only some level of disruption i.e. FOT tokens not supported and no funds at risk)

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.

Appeal created

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