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

DoS on DeliveryPlace::settleAskTaker call

radin100

Summary

When user manages to settle points via DeliveryPlace::settleAskTaker, he will face Unauthorized revert, due to bad msg.sender validation

Vulnerability Details

Here common scenario:

  1. Alice creates bidOffer with PreMarkets::createOffer, then Bob creates askOrder by calling PreMarkets::createTaker, which corresponds with Alice's offer and now Bob becomes taker.

  2. Now Alice is authority in offerInfo type Bid and Bob is authority in stakeInfo type Ask.

  3. After some time block.timestamp reaches provided token's tge value and as we know from the docs after the tge (during the settlementPeriod),now Bob is supposed to settle Alice's points by calling DeliveryPlace::settleAskTaker

Docs scenario:

  • As the Original Offer Maker, Alice posts a buy offer for 1,000 points with a unit price of $1 and pays $1,000 in advance to the platform's smart contract.

  • Bob accepts Alice's buy offer and sells 1,000 points to Alice. This transaction results in Bob having a sell-type inventory in his account.

  • Bob will settle 1000 points to Alice after the Token Generation Event (TGE) of the token.

But unawares, during the DeliveryPlace::settleAskTaker execution, the function call is going to revert Unauthorized, due to msg.sender(stockAuth) != offerInfo.authority, which is not expected to happen, due to provided invariant

/**
* @notice Settle ask taker
-> * @dev caller must be stock authority
* @dev market place status must be AskSettling
* @param _stock stock address
* @param _settledPoints settled points
* @notice _settledPoints must be less than or equal to stock points
*/
function settleAskTaker(address _stock, uint256 _settledPoints) external {
...
​
if (status == MarketPlaceStatus.AskSettling) {
-> if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
​
...
}

Note that a comment above settleAskTaker says: * @dev caller must be stock authority and confirms the invariant

Impact

Core functionality broke, DoS when taker manages to settle points from an order

Tools Used

Manual review

Recommendations

function settleAskTaker(address _stock, uint256 _settledPoints) external {
...
​
if (status == MarketPlaceStatus.AskSettling) {
- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
​
...
}
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!