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

An Ask Turbo Offer can be aborted even if there is sub offer listed

h2134

Summary

An Ask Turbo Offer can be aborted even if there is sub offer listed, due to AbortOfferStatus is not updated as expected.

Vulnerability Details

When an ask Turbo offer is filled, the taker can list a sub offer of the origin offer. In that case, the origin offer is not supposed to be aborted.

if (offerInfo.abortOfferStatus != AbortOfferStatus.Initialized) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Initialized,
offerInfo.abortOfferStatus
);
}

The offer's AbortOfferStatus is updated when the sub offer is listed:

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

However, if we pay a close attention, we can find that the offer's AbortOfferStatus is not actually updated, because the data location of originOfferInfo is memory rather than storage.

OfferInfo memory originOfferInfo = offerInfoMap[originOffer];

As a result, the origin offer authority and sub offer authority can call to abort, but the taker of the sub offer cannot abort their stock and their collaterals will be stuck. This is because to abort bid taker, the AbortOfferStatus of the stock's pre offer must be Aborted:

if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}

Unfortunately, this status is only updated to Aborted when abortAskOffer is called on the origin offer, meaning AbortOfferStatus of any sub offer won't be updated, despite the sub offer has already been aborted.

Imagine the following scenario:

  1. Alice creates an ask offer, OfferSettleType is Turbo;

  2. Bob creates an taker against the offer, and sublist a sub offer, preOfferInfo is Alice's origin offer;

  3. Cathy fills bob's sub offer, preOfferInfo is Bob's sub offer;

  4. Because the origin offer's AbortOfferStatus is not actually updated, Alice is able to abort the offer and receive refund, the origin offer's abortOfferStatus is updated to Aborted;

  5. Bob calls to abort bid taker, also receives refund, the sub offer's abortOfferStatus is not updated;

  6. However, when Cathy calls to abort bid taker, the transaction will be reverted because the sub offer's abortOfferStatus is NOT Aborted.

Please copy the PoC code into PreMarkets.t.sol to verify:

function testAudit_OfferCanBeAbortedEvenIfSubOfferIsListed() public {
// Create Market
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
vm.stopPrank();
​
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1200e18);
​
address bob = makeAddr("Bob");
deal(address(mockUSDCToken), bob, 103.5e18);
​
address cathy = makeAddr("Cathy");
deal(address(mockUSDCToken), cathy, 103.5e18);
​
address aliceOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
address aliceStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
​
// Alice creates an ask offer
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(CreateOfferParams({
marketPlace: auditMarketPlace,
tokenAddress: address(mockUSDCToken),
points: 1000,
amount: 1000e18,
collateralRate: 12000,
eachTradeTax: 300,
offerType: OfferType.Ask,
offerSettleType: OfferSettleType.Turbo
}));
vm.stopPrank();
​
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
address bobOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Bob creates taker to buy Alice's offer and list stock
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceOfferAddr, 100);
preMarktes.listOffer(bobStockAddr, 100e18, 12_000);
vm.stopPrank();
​
address cathyStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
address cathyOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Cathy creates taker to buy Bob's offer
vm.startPrank(cathy);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(bobOfferAddr, 100);
vm.stopPrank();
​
// Alice aborts offer
vm.prank(alice);
preMarktes.abortAskOffer(aliceStockAddr, aliceOfferAddr);
​
// Bob aborts taker
StockInfo memory bobStockInfo = preMarktes.getStockInfo(bobStockAddr);
vm.prank(bob);
preMarktes.abortBidTaker(bobStockAddr, bobStockInfo.preOffer);
​
// Cathy tries to abort but failed
StockInfo memory cathyStockInfo = preMarktes.getStockInfo(cathyStockAddr);
vm.prank(cathy);
vm.expectRevert(abi.encodeWithSelector(IPerMarkets.InvalidAbortOfferStatus.selector, uint8(AbortOfferStatus.Aborted), uint8(AbortOfferStatus.Initialized)));
preMarktes.abortBidTaker(cathyStockAddr, cathyStockInfo.preOffer);
}

Impact

Offer can be aborted even if there is sub offer listed, and the taker who fill the sub offer will lose their collaterals.

Tools Used

Manual Review

Recommendations

Change origin offer's data location to `storage` when update its AbortOfferStatus:

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