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

Access control vulnerability in `DeliveryPlace.settleAskTaker()` function leading to potential loss of funds.

eeyore

Summary

The offerInfo.authority is incorrectly allowed to call the DeliveryPlace.settleAskTaker() function when, in fact, only the stockInfo.authority should be permitted.

Vulnerability Details

In the DeliveryPlace.settleAskTaker() function, the access control check is implemented as follows:

File: DeliveryPlace.sol
360: if (status == MarketPlaceStatus.AskSettling) {
361: if (_msgSender() != offerInfo.authority) { // <== Invalid check, this should be compared with stockInfo.authority
362: revert Errors.Unauthorized();
363: }

The settleAskTaker() function is intended to be used by the Taker of a Bid offer to settle Points with the Maker of that offer.

However, with the current access control, the function forces the Maker to transfer Points tokens to themselves as part of the settlement, as shown below:

File: DeliveryPlace.sol
377: tokenManager.tillIn(
378: _msgSender(), // <== Maker is forced to transfer tokens they think they bought
379: marketPlaceInfo.tokenAddress,
380: settledPointTokenAmount,
381: true
382: );

This issue is problematic because it allows the Maker to craft a Bid offer that can result in stealing funds from any Taker of such an offer. Here is how this can happen:

  1. The Maker creates a Bid offer stating that they want to buy X Points for 1000 USDC and sets the collateral factor to 200%.

  2. A Taker accepts the Bid offer and transfers 2000 USDC as collateral in exchange for 1000 USDC and a promise that they will settle X Points with the Maker after the TGE.

  3. After the TGE, the Taker is unable to settle because they do not have access to the settleAskTaker() function. Instead, the Maker calls the function with a 0 amount, stealing all the collateral and leaving the Taker with a 1000 USDC loss.

Impact

  • Incorrect access control.

  • Loss of funds.

Tools Used

Manual review.

Recommendations

Fix the access control check:

File: DeliveryPlace.sol
360: if (status == MarketPlaceStatus.AskSettling) {
-361: if (_msgSender() != offerInfo.authority) {
+361: if (_msgSender() != stockInfo.authority) {
362: revert Errors.Unauthorized();
363: }
Updates

Lead Judging Commences

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