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

offer.abortOfferStatus is never changed but it's expected to be

dinkras

Summary

Vulnerability Details

All turbo offers have the ability to have subsequent offers without collateral according to the Tadle docs. https://tadle.gitbook.io/tadle/how-tadle-works/mechanics-of-tadle/turbo-mode

When lPreMarkets.istOffer() is being called, there is a if statement that checks if offerSettleType is Turbo, the origin offer abortOfferStatus is expected to be changed to SubOfferListed. However the state var is not updated, instead the change is done only in memory, instead of storage.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L336-L342

if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;//@audit-issue abortOfferStatus not updated in storage. should be storage instead of memory:
}

As a result, the offer info state var stays with an incorrect value.

Impact

Incorrect state of offer.abortOfferStatus

Tools Used

Manual review

Recommendations

Change the variable to storage here:

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L337C48-L337C73

- OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
+ OfferInfo storage originOfferInfo = offerInfoMap[originOffer];
​
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.