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

State change to memory variable in PreMarkets::listOffer  will fail to persist

0x5chn0uf

Summary

In the PreMarkets::listOffer function there's an attempt to update the abortOfferStatus of the original offer when the offer settlement type is Turbo.

However, this update is performed on a memory variable (originOfferInfo) instead of a storage variable.

As a result, the state change does not persist, leading to a failure in updating the abort offer status of the original offer.

Vulnerability Details

if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
@> OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
@> originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}

The following test demonstrates the value is not updated:

function test_memoryVariableDoesNotModifyStorage() public {
vm.startPrank(user1);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo // Settle Type is Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
// List the Offer
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
// Check the status of the original offer
OfferInfo memory originalOffer = preMarktes.getOfferInfo(
offerAddr
);
// This assertion passes, but it shouldn't
assert(originalOffer.abortOfferStatus == AbortOfferStatus.Initialized);
​
vm.stopPrank();
}

Impact

This bug prevents the proper updating of the original offer's status when a sub-offer is listed in Turbo settlement mode.

It disrupts the intended flow of the Turbo settlement process.

Tools Used

Manual Review - Testing

Recommendations

Modify the listOffer function to update the storage variable directly:

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;
}

By changing memory to storage, we ensure that the modification is made directly to the contract's state, persisting the change to the original offer's status.

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!