Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

An origin ask turbo offer cannot be aborted even if there is no sub offer listed

h2134

Summary

An origin ask turbo offer cannot be aborted even if there is no sub offer listed.

Vulnerability Details

When an ask offer is created, its abortOfferStatus is initialized to AbortOfferStatus.Initialized. The offer's abortOfferStatus will be updated to AbortOfferStatus.SubOfferListed when a sub offer is listed (actually there is an issue and we will fix it in the PoC):

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 authority is supposed to be able to abort the offer if there is no sub offer listed, or the transaction will be reverted:

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

However, it's not always the case. After a sub offer is listed, it can also be closed by the authority, the problem is that the abortOfferStatus of the origin offer is not updated accordingly, the origin offer's abortOfferStatus remains AbortOfferStatus.SubOfferListed hence it cannot be aborted even if there is sub offer listed.

Imagine the following scenario:

  1. Alice creates an ask turbo offer;

  2. Bob creates a bid taker against the ask offer

  3. Bob lists the sub offer;

  4. Alice cannot abort the orgin ask offer because there is sub offer listed;

  5. Bob closes the sub offer;

  6. Alice still cannot abort the origin ask offer even if there is no sub offer listed, due to that the abortOfferStatus of the origin offer is not updated and remains AbortOfferStatus.SubOfferListed.

There is another issue which is closely related to this issue: When a sub offer is closed, user can relist the offer, but the abortOfferStatus of the origin offer is not updated to AbortOfferStatus.SubOfferListed, currently there is no impact, but if we are to fix the close issue by updating the abortOfferStatus, we should also fix this relist issue, otherwise the the origin offer can be aborted despite there are sub offers listed.

Please follow the steps below to run the PoC:

  • Change Line 337 as below (this is to fix another issue in the codebase):

- OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
+ OfferInfo storage originOfferInfo = offerInfoMap[originOffer];

  • Run the test case below in PreMarkets.t.sol to verify:

function testAudit_OfferCannotBeAbortedEvenIfThereIsNoSubOfferListed() 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 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 bobOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
​
// Bob creates taker
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceOfferAddr, 100);
vm.stopPrank();
​
// Bob lists a sub offer
vm.prank(bob);
preMarktes.listOffer(bobStockAddr, 100e18, 12000);
​
// Alice tries to abort the origin offer but failed because there is sub offer listed
vm.prank(alice);
vm.expectRevert(abi.encodeWithSelector(IPerMarkets.InvalidAbortOfferStatus.selector, uint8(AbortOfferStatus.Initialized), uint8(AbortOfferStatus.SubOfferListed)));
preMarktes.abortAskOffer(aliceStockAddr, aliceOfferAddr);
​
// Bob closes the listed sub offer
vm.prank(bob);
preMarktes.closeOffer(bobStockAddr, bobOfferAddr);
​
// Alice still cannot abort the origin offer even if there is no sub offer listed
vm.prank(alice);
vm.expectRevert(abi.encodeWithSelector(IPerMarkets.InvalidAbortOfferStatus.selector, uint8(AbortOfferStatus.Initialized), uint8(AbortOfferStatus.SubOfferListed)));
preMarktes.abortAskOffer(aliceStockAddr, aliceOfferAddr);
}

Impact

Ask turbo Offer cannot be aborted even if there is no sub offer listed.

Tools Used

Manual Review

Recommendations

When sub offers are closed, the original offer's abortOfferStatus should be updated accordingly, the mitigation can be complicated because all the sub (nested / sold) offers should be taken into consideration when implement the fixing.

It is much easier to fix the relist issue, simply update the original offer's abortOfferStatus to AbortOfferStatus.SubOfferListed at the end of the call of relistOffer().

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

[invalid] finding-PreMarkets-abortAskOffer-revert-SubOfferListed

This findings hinges on the fix to #1616. Since it DoSes abortAskOffer that should be allowed, medium severity seems appropriate. Note for invalidation: This highlights a potential inconsistency that sellers should be able to cancel their orders at any time if they are not fulfilled and retrieve their collateral as noted in the [doc](https://tadle.gitbook.io/tadle/product/points-marketplace#how-tadle-unlock-points-liquidity-on-our-marketplace). However, not how it mentions `if applicable` . > Upon completing the trade, the seller will receive the funds instantly, without having to wait for the token unlock at TGE. Sellers can cancel their orders at any time if they are not fulfilled and retrieve their collateral, if applicable. So one can interpret it as once a bid taker offer is created against an orign maker ask offer, the revenue earned represented by SalesRevenue (see issue #826 and #765 for more indepth explanation) is assigned to the origin maker that can be withdrawed immediately (meaning order is already fulfilled). So arguably, in turbo mode, once an sub offer is listed, it is an acceptable design decision to disallow termination even if the suboffer is not settled, given taker should be the rightful holder of the 100 points (per the PoC) i.e. the maker should settle, if not they can walk away with the collateral paid by taker FOC. I believe this issue is invalid

Appeal created

h2134 Submitter
over 1 year ago
0xbrivan2 Judge
over 1 year ago
h2134 Submitter
over 1 year ago
0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

[invalid] finding-PreMarkets-abortAskOffer-revert-SubOfferListed

This findings hinges on the fix to #1616. Since it DoSes abortAskOffer that should be allowed, medium severity seems appropriate. Note for invalidation: This highlights a potential inconsistency that sellers should be able to cancel their orders at any time if they are not fulfilled and retrieve their collateral as noted in the [doc](https://tadle.gitbook.io/tadle/product/points-marketplace#how-tadle-unlock-points-liquidity-on-our-marketplace). However, not how it mentions `if applicable` . > Upon completing the trade, the seller will receive the funds instantly, without having to wait for the token unlock at TGE. Sellers can cancel their orders at any time if they are not fulfilled and retrieve their collateral, if applicable. So one can interpret it as once a bid taker offer is created against an orign maker ask offer, the revenue earned represented by SalesRevenue (see issue #826 and #765 for more indepth explanation) is assigned to the origin maker that can be withdrawed immediately (meaning order is already fulfilled). So arguably, in turbo mode, once an sub offer is listed, it is an acceptable design decision to disallow termination even if the suboffer is not settled, given taker should be the rightful holder of the 100 points (per the PoC) i.e. the maker should settle, if not they can walk away with the collateral paid by taker FOC. I believe this issue is invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!