Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Wrong comments for abortBidtaker function

VaRuN

Summary

Vulnerability Details

Following is abort bid taker function

/**
* @notice abort bid taker
* @param _stock stock address
* @param _offer offer address
* @notice Only offer owner can abort bid taker
* @dev Only offer abort status is aborted can be aborted
* @dev Update stock authority refund amount
*/
function abortBidTaker(address _stock, address _offer) external {
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage preOfferInfo = offerInfoMap[_offer];
​
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
​
if (stockInfo.preOffer != _offer) {
revert InvalidOfferAccount(stockInfo.preOffer, _offer);
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus(
StockStatus.Initialized,
stockInfo.stockStatus
);
}
​
if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}
​
uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points,
preOfferInfo.amount,
Math.Rounding.Floor
);
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
​
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);
​
stockInfo.stockStatus = StockStatus.Finished;
​
emit AbortBidTaker(_offer, _msgSender());
​
}

As can be seen that the comments state that the function can only be called by the offer owner but instead in the function it allows only stock owner to call this function.

* @notice Only offer owner can abort bid taker
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}

Now either comments are wrong or the implementation is wrong . According to me the comments are wrong thus low severity otherwise high severity. Why I think that the comment is wrong is because once the owner of a offer aborts the ask offer than the bidder for that order should be allowed to get his funds back and thats exactly what should happen in abortBidTaker function hence stock owner should only be allowed to call this function.

Impact

Either wrong owner is allowed to call this function or comments are wrong.

Tools Used

Manual review.

Recommendations

Either update the comments or change the access control of the funciton.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-abortBidTaker-wrong-stock-authority

Invalid. 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). Hence, we should verify `stockInfoMap`, regardless of the taker order being a ASK (selling points) or BID (buying points) taker order, so there is no issue here, other than documentation error

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.