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

The AbortBidTaker function allows the user to abort any order at any moment

mikebello

Summary

The AbortBidTaker function doesn't do the same checks as the abortAskOffer function regarding the market status and the offer type, this allow the user to call AbortBidTaker at any moment, even when the market is offline.

Vulnerability Details

The abortAskOffer y AbortBidTaker are functions that the user can call to cancel his respective offers and avoid settlement, this functions have to validate the orders are in certain states before allowing the user to abort his orders and return them their tokens.

The abortAskOffer function do many checks prior to allow the user to abort his ask offer, some of the checks are:

function abortAskOffer(address _stock, address _offer) external {
//..
if (offerInfo.offerType != OfferType.Ask) {
revert InvalidOfferType(OfferType.Ask, offerInfo.offerType);
}
//...
/// @dev market place must be online
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
//...
}

The AbortBidTaker do similar validations to ensure the order the user is trying to abort is in the correct state to allow the user to abort his order, but this function lacks the validation of the stockType and the MarketPlaceStatus like the abortAskOffer does, this lack of validation allows any user to abort an order without checking the market status, so the user can abort an order on any state of the market, even when the market is offline.

also the stockType order is not validated, so an user can abort an ask and a bid taker orders.

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

To show that a user can abort an order in any market state I added this test to the PreMarkets.t.sol, the test should revert but id doesn't.

function test_User_can_Abort_atAny_MarketState() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e6, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
preMarktes.createTaker(offerAddr, 500);
vm.stopPrank();
​
vm.prank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
// market place is Offline
MarketPlaceStatus offline_status = MarketPlaceStatus.Offline;
vm.startPrank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
// set the market as offline
systemConfig.updateMarketPlaceStatus("Backpack", offline_status);
vm.stopPrank();
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
​
// The user can abort a bid taker even when the market is Offline
// this should fail, but it doesn't
vm.startPrank(user1);
preMarktes.abortBidTaker(stock1Addr, offerAddr);
vm.stopPrank();
}

Impact

users can abort Bid and Ask taker orders any moment, it doesn't matter the status of the market place, base con the code, the user should be able to abort only Bid taker orders when the market is online.

Tools Used

Manual Review.

Recommendations

Implement the complementary checks in the AbortBidTaker for ensure the users can only abort a bid taker order and also validate that the market is in the required state.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] finding-PreMarkets-abortBidTaker-lack-check-abort

Informational, during emergencies, even if abortions are allowed, withdrawal can be paused and collateral cannot be pulled anyways (`whenNotPaused` modifier within `withdraw()`), so there is no impact here, given funds outflow can be paused.

Support

FAQs

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