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

Ineffective state update in Turbo Mode Offer Listing leading to incorrect abortOfferStatus of origin offer

tinnohofficial

Summary

In the PreMarktes::listOffer function, when a taker lists an offer in Turbo mode, the origin offer's abortOfferStatus is not properly updated in storage. This leads to potential inconsistencies in the contract state

Vulnerability Details

When attempting to update the abortOfferStatus of the origin offer, memory keyword creates a local copy of the offer info, so modifications to originOfferInfo do not persist in the storage.

function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable {
//...
​
/// @dev change abort offer status when offer settle type is turbo
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
// @audit should be storage since we are changing abortOfferStatus
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
​
// @audit since it is memory this doesn't do any change
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
​

Impact

The originOffer abort status is in incorrect state causing issues in other parts of system (and potentially future upgrades).
For example in current implementation of PreMarktes::abortAskOffer() the check if (offerInfo.abortOfferStatus != AbortOfferStatus.Initialized) will still be BYPASSED by the originOffer since its abort offer status was never changed (when the taker listed the offer via PreMarktes::listOffer).

Tools Used

Manulal review

Recommendations

Change the declaration of originOfferInfo to use storage

function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable {
//...
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 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!