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

Broken access control in DeliveryPlace.settleAskTaker() leads to loss of funds

ge6a

Summary

The DeliveryPlace.settleAskTaker function is called by the taker who has been matched with a bid maker to provide the necessary tokens to complete the transaction. The problem is that authorization is performed by checking whether _msgSender() is equal to the authority of the offer. This is incorrect because the authority of the offer wants to acquire tokens at a certain price and therefore wouldn't have them to settle the position. The correct approach (as stated in a comment in the code) is to check whether _msgSender() is equal to the authority of the stock. This issue prevents the taker from settling the transaction, leading to a loss of funds for the taker due to the loss of the collateral that was deposited when the taker was created.

Vulnerability Details

if (status == MarketPlaceStatus.AskSettling) {
@>if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}

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

POC

function test_revert_create_bid_offer_turbo_usdc() public {
vm.startPrank(user);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
vm.prank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
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(user2);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
​
deliveryPlace.settleAskTaker(stock1Addr, 500); // this will revert
vm.stopPrank();
}

Impact

Loss of funds for the taker and broken core functionality.

Tools Used

Manual review

Recommendations

Adjust the code as described in the report above.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-settleAskTaker-wrong-stock-authority

Valid high severity, when taker offers are created pointing to a `offer`, the relevant `stockInfoMap` offers are created with the owner of the offer aka `authority`, set as the creater of the offer, as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L245). Because of the wrong check within settleAskTaker, it will permanently DoS the final settlement functionality for taker offers for the maker that listed the original offer, essentially bricking the whole functionality of the market i.e. maker will always get refunded the original collateral, and takers will never be able to transact the original points put up by the maker. This occurs regardless of market mode.

Support

FAQs

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

Give us feedback!