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

Incorrect authorization check in `settleAskTaker()` prevents settlement and leads to loss of points

0xnbvc

Vulnerability Details

The DeliveryPlace contract contains the function settleAskTaker() which is responsible for settling Ask takers. This function checks if the caller is authorized to settle the Ask taker by comparing the caller's address with the authority field of the offer.
However, this check should be performed against the authority field of the stock instead of the offer.

DeliveryPlace.sol#L360-L363

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

Therefore, the taker won't be able to settle during the AskSettling period, and once the period is done, even the admin (owner) won't be able to settle it if _settledPoints > 0, thus the points will be lost.

DeliveryPlace.sol#L364-L371

} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
>> if (_settledPoints > 0) {
>> revert InvalidPoints();
}
}

Impact

Takers are unable to settle Ask takers during the AskSettling period leading to loss of funds.

Proof of Concept

  • Add this PoC to test/PreMarkets.t.sol

  • Add import {Errors} from "src/utils/Errors.sol"; at the top of the file

  • Run forge test --mt test_PoC_settleAskTaker -vvvv

function test_PoC_settleAskTaker() public {
// user creates an offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
​
// user2 creates a taker for the offer
vm.startPrank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
// Admin updates the market
vm.startPrank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
// user2 tries to settle his Ask Taker but it reverts due to the check
vm.startPrank(user2);
address stock1Addr = GenerateAddress.generateStockAddress(1);
// Expect the settleAskTaker call to revert with Unauthorized error
vm.expectRevert(Errors.Unauthorized.selector);
deliveryPlace.settleAskTaker(stock1Addr, 500);
}

Recommendations

To resolve this issue, update the authorization check in settleAskTaker() to compare the caller's address with the authority field of the stock instead of the offer.

if (status == MarketPlaceStatus.AskSettling) {
- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
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.