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

Partial Settlements Never Work and Instead Revert with an Unauthorized Error

bauchibred

Summary

Unlike full settlements, when there are multiple people placing bids, the settlement fails for the user. This can be proven by running the test_partial_settlement test:

Vulnerability Details

See the settleAskMaker function in the DeliveryPlace contract: https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L222-L325.

Now attach this POC to the end of https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/test/PreMarkets.t.sol

function test_partial_settlement() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
​
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 300);
vm.stopPrank();
​
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 400);
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.prank(user);
mockPointToken.approve(address(tokenManager), type(uint256).max); // Ensure sufficient allowance for settlement
// Expect an Unauthorized revert when user tries to settle the Ask offer
vm.expectRevert(0x82b42900); // 0x82b42900 = keccak 'Unauthorized()'
deliveryPlace.settleAskMaker(offerAddr, 500);
// Check remaining balance
OfferInfo memory offerInfo = preMarktes.getOfferInfo(offerAddr);
assertEq(offerInfo.usedPoints, 700);
assertEq(offerInfo.settledPoints, 0); // Expect settledPoints to be 0 since settlement failed
}

As shown by the test, whereas all other instances of this never fail, i.e calling settleAsMaker() in the case where we are attempting a partial settlement, the function reverts with an "Unauthorized" error.

Impact

Users who have placed bids may have their funds locked in the contract, unable to be settled or withdrawn, leading to potential financial losses. Also the inability to partially settle offers can lead to market inefficiencies, as large offers may remain unsettled for longer periods.

Tools Used

Manual review

Recommendations

Consider handlinng partial settlements correctly. This should include updating the offer's state to reflect partial settlements and ensuring that funds are distributed proportionally.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.