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 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!