Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

`PreMarkets.listOffer()` doesn't update the `abortOfferStatus` properly.

bladesec

Summary

PreMarkets.listOffer() doesn't update the abortOfferStatus of originOfferInfo properly.

Vulnerability Details

In listOffer(), it updates the originOfferInfo.abortOfferStatus to SubOfferListed for the Turbo settle type.

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

But it won't be updated properly because originOfferInfo is a memory variable instead of storage.

Impact

The originOfferInfo.abortOfferStatus wouldn't be updated properly after calling listOffer().

Tools Used

Manual Review

Recommendations

originOfferInfo should be declared as storage.

/// @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;
}
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.