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 about 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.