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

In the function `PreMarkets::listOffer()` a status update is done to a memory instance rather than to the storage variable

Chad0

Summary

In the function PreMarkets::listOffer() the update to AbortOfferStatus is done on a memory instance originOfferInfo, which is not stored back into the storage of offerInfoMap. Therefore, this update doesn't persist in the blockchain state, making it ineffective.

Vulnerability Details

The vulnerable code are these lines, quoted below:

if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}

Apparently the update is done to the memory var of originOfferInfo, and not in storage.

Impact

This issue leads to the consequence that the offerInfo of the originOffer, although should have become SubOfferListed for its abortOfferStatus, remains Initialized. This will mess up a lot of the protocol's logic relying on the correct accounting of AbortOfferStatus. For example, such origin offers are still eligible to be aborted, although it should not have been allowed to, because there is already sub offer listed. Since it can mess up the protocol's intended design and user's fund flow associated with it, this should be a H severity issue.

Tools Used

Manual Review

Recommendations

Update the status to the storage, rather than on the memory instance only.

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.