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

Incorrect memory assignment fails to update originOfferInfo.abortOfferStatus properly in listOffer() function

matejdb

Summary

Variable originOfferInfo.abortOfferStatus not updated properly (in storage) due to a faulty memory assignment when retrieving originOfferInfo from storage mapping.

Vulnerability Details

The variable is only changed in memory - not in storage - so the update is not stored properly.

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

Impact

abortOfferStatus of Turbo offers is not properly stored on-chain. This can cause some checks to pass correctly even though they should not.

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

Tools Used

Manual review

Recommendations

Change memory assignment of variable to storage:

/// @dev change abort offer status when offer settle type is turbo
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo storage originOfferInfo = offerInfoMap[originOffer]; // @audit-info
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
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.