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

Incorrect Memory Update and Inconsistent Logic in abortOfferStatus Handling

cryptomoon

Summary

The current implementation of the abortOfferStatus update(in listOfferfunction) for originOfferInfo contains critical issues. The status is incorrectly updated in memory instead of storage.

Vulnerability Details

The abortOfferStatus is being updated in memory instead of storage, leading to the update not being persisted. This results in the status not reflecting the actual state of the offer after the function execution.

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

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

Add the following POC in PreMarkets.t.sol:

function test_turbo_offer_abort_subOfferrelisted() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address offer1Addr = GenerateAddress.generateOfferAddress(0);
address stock1Addr = GenerateAddress.generateStockAddress(0);
preMarktes.createTaker(offer1Addr, 500);
​
address stock2Addr = GenerateAddress.generateStockAddress(1);
address offer2Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.listOffer(stock2Addr, 0.006 * 1e18, 12000);
preMarktes.createTaker(offer2Addr, 500);
// WILL NOT REVERT
preMarktes.abortAskOffer(stock1Addr, offer1Addr);

Impact

Due to above issue as it is not updating the storage, the turbosettlementType offers which will have subOfferlisted can also be aborted by calling abortAskMaker. This will cause loss of funds for the protocol since in turbomode, subOffers are not collateralized and due to abortof offer, they will get the collateral.

Consider following scenario to understand the impact:

  • Maker M1listed 100 points for 100 USDCin turbo mode

  • Taker T1bought 100 points for 100 USDCfrom M1

  • Taker T1listed the 100 points for 200 USDC

  • Buyer B1bought 100 points for 200 USDCfrom T1.

If M1calls abortAskOfferand aborts the offer and take the collateral back, who will settle the stockof Buyer B1.

Tools Used

Manual review

Recommendations

Fixed code:

/// @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 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.