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

abortOfferStatus will not be updated at PreMarkets.sol::listOffer() since the struct is loaded into memory instead of storage

tamer

Summary

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L342

originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed; will not get updated since it is loaded from memory, instead of storage.

Vulnerability Details

It's not updating the abortOfferStatus.

Impact

When the problem gets fixed, it will open a new problem:

It could lead to DoS inside of PreMarkets.sol::abortAskOffer() when checking for the abortOfferStatus since the abortOfferStatus could also be SubOfferListed

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

Tools Used

Manual Review

Recommendations

Change loading to storage instead of to memory.

/// @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];
+ OfferInfo storage originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
​
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}

Consider a different approach at abortAskOffer() for the abortOfferStatus check.

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!