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

Collateral fees are stuck when Admin settles ASK Order instead of Taker

vinica_boy

Summary

When the Admin settles on behalf of an ASK Taker, who is responsible for providing tokens after placing a BID Offer, the collateral provided by the Taker becomes stuck in the protocol. This happens because the current implementation lacks a mechanism for the Admin to withdraw the collateral, leaving the funds trapped.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L335
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L368
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L376
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L400

Admin needs to call settleAskTaker() in DeliveryPlace.sol so the BID Maker is able to call closeBidOffer() in the same contract. Logic in the settleAskTaker() does not account the collateral provided by the ASK Taker.

On line #368, the condition if (_settledPoints > 0) will always evaluate to false because the only possible value for _settledPoints is 0. This causes the subsequent conditions on line #376 (if (settledPointTokenAmount > 0)) and line #400 (if (_settledPoints == stockInfo.points)) to also fail. As a result, the protocol fails to account for the collateral that the Admin should receive from the ASK Taker, leading to unaccounted and stuck funds.

The following test case shows the described scenario:

function testAdminSettleAskTaker() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Protected
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
vm.prank(user2);
preMarktes.createTaker(offerAddr, 500);
​
vm.startPrank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
// Admin settles instead of ASK Taker after settlement period is finished
// warp to set timestamp after settlement period is finished
vm.warp(1719826111275);
deliveryPlace.settleAskTaker(stock1Addr, 0);
vm.stopPrank();
​
vm.startPrank(user);
deliveryPlace.closeBidOffer(offerAddr);
// BID Maker has received back his collateral + fee
console.log(tokenManager.userTokenBalanceMap(address(user), address(mockUSDCToken), TokenBalanceType.MakerRefund));
// Admin is not accounted for the collateral provided by ASK Taker.
console.log(tokenManager.userTokenBalanceMap(address(user1), address(mockUSDCToken), TokenBalanceType.RemainingCash));
vm.stopPrank();
}

Impact

Funds stuck in the contract due to wrong accounting.

Tools Used

Manual review, Foundry.

Recommendations

Consider adding logic to ensure that when the Admin settles, the Admin is properly accounted for and can withdraw the funds provided by the party that failed to settle (in this case, the ASK Taker).

Updates

Lead Judging Commences

0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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