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

Wrong `msg.sender` validation used in the function `DeliveryPlace::settleAskTaker()`

Chad0

Summary

The function DeliveryPlace::settleAskTaker() is supposed to be called by the Taker who has placed an Ask Order; however, the caller validation specifies it to be called by the offerInfo.authority in the normal case (if (status == MarketPlaceStatus.AskSettling)), which is referring to the Maker of the Bid Offer but not the Taker. Thus, the caller validation used is totally wrong.

Vulnerability Details

The impacted lines of code are these.

Quoted here:

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

Instead of offerInfo.authority, it should use stockInfo.authority.

Even in the Line#329, the dev himself wrote:

* @dev caller must be stock authority

Impact

With this flipped caller of the function, this function will be totally messed up. The intended settlement will never work, because the caller Maker is actually the buyer side of the trade and he does not have any required token to settle. This trade goes to the direction of collateral liquidation even though the real Taker may actually have the willingness and capability to deliver the settlement in a good way. This issue is absolutely H severity.

Tools Used

Manual Review

Recommendations

Revise the buggy code

if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}

to this code

if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
Updates

Lead Judging Commences

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