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

Unable to set AbortOfferStatus.SubOfferListed status

p0wd3r

Summary

Due to the incorrect use of memory instead of storage in the code, the AbortOfferStatus.SubOfferListed set in listOffer cannot be saved.

Vulnerability Details

The following code is in listOffer:

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L334-L343

/// @dev change abort offer status when offer settle type is turbo
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
//@audit memory, not storage
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}

The issue here is that originOfferInfo is a memory variable, and its setting will become invalid after the transaction is completed and will not be saved. Therefore, the subsequent setting of AbortOfferStatus.SubOfferListed has no meaning and cannot be saved.

Impact

The AbortOfferStatus.SubOfferListed set in listOffer cannot be saved.

If an offer has sub offers, it cannot be aborted, but this bug prevents the offer from recording whether it has sub offers or not.

if (offerInfo.abortOfferStatus != AbortOfferStatus.Initialized) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Initialized,
offerInfo.abortOfferStatus
);
}

Tools Used

vscode

Recommendations

memory-> storage

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-listOffer-originIOfferInfo-storage-memory

Valid high severity, because the `abortOfferStatus` of the offer is not updated and persist through `storage` when listing an offer for turbo mode within the `offerInfoMap` mapping, it allows premature abortion given the `abortOfferStatus` defaults to `Initialized`, allowing the bypass of this [check](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L552-L557) here and allow complete refund of initial collateral + stealing of trade tax which can potentially be gamed for profits using multiple addresses

Support

FAQs

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

Give us feedback!