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

Authority is not checked correctly at settleAskTaker

matrox

Summary

contract: DeliveryPlace.sol, function: settleAskTaker
According to the program logic and the function comments, the authority of the sender should be checked against the stock authority, however it is checked against offer authority.

Vulnerability Details

Since the taker should call settleAskTaker, we should check the _msgSender to be the stock authority. However, at settleAskTaker, the offer's authority is checked which is incorrect and causing the function proceed only when the taker is the same as the maker, otherwise it reverts.

if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != offerInfo.authority) { // FOUND: this should be: _msgSender() != stockInfo.authority
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}

Impact

If the taker is different than the maker user, which is normally the case, the function reverts. In this case, the taker cannot transfer the points to the maker who has made a bid on, and the maker's 'buy' process will fail.

Tools Used

Manual Review

Recommendations

Please change the authority check condition to:

if (status == MarketPlaceStatus.AskSettling) {
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 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.