Tadle

Tadle
DeFi
30,000 USDC
View results
Submission Details
Severity: low
Invalid

Users with created offers will lose their funds when market is disabled

Summary

The admin from Tadle can disable a market at any time without any restriction.

/**
* @notice Update market place status
* @param _marketPlaceName Market place name
* @param _status Market place status
* @notice Caller must be owner
*/
function updateMarketPlaceStatus(
string calldata _marketPlaceName,
MarketPlaceStatus _status
) external onlyOwner {
@> address marketPlace = GenerateAddress.generateMarketPlaceAddress(
_marketPlaceName
);
MarketPlaceInfo storage marketPlaceInfo = marketPlaceInfoMap[
marketPlace
];
@> marketPlaceInfo.status = _status;
}

The problem here is that all the operations from the PreMarket including closeOffer(one of the actions that makers can take to rescue their funds) are dependent on the market status == online.

Basically disabling a market with active makers/takers will cause DoS to withdraw the funds of those users.

i.e: closeOfferthe function responsible to prepare the collateral to be withdrawn:

function closeOffer(address _stock, address _offer) external {
OfferInfo storage offerInfo = offerInfoMap[_offer];
StockInfo storage stockInfo = stockInfoMap[_stock];
...
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
@> marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
@> MarketPlaceStatus.Online
);
...
}

Impact

  • The protocol will block users from withdrawing their funds. All the collateral will get locked into the protocol.

  • The locked funds combined with the function rescueput the trust in the protocol at risk. Explanation below:

    Basically, it gives authority to the protocol to lock the funds and withdraw all the tokens deposited by the users. As the rescuefunction gives the possibility to withdraw any amount at any time.

// rescue
function rescue(
address to,
address token,
uint256 amount
@> ) external onlyOwner {
if (token == address(0x0)) {
@> payable(to).transfer(amount);
} else {
@> _safe_transfer(token, to, amount);
}
emit Rescue(to, token, amount);
}

Tools Used

Manual Review

Recommendations

  1. Users from disabled markets should be able to close their offers and withdraw their funds.

  2. Consider adding a verification to check whether the market meets the criteria to be disabled.(i.e: doesn't have any pending offers)

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-Admin-Errors-Malicious

The following issues and its duplicates are invalid as admin errors/input validation/malicious intents are1 generally considered invalid based on [codehawks guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). If they deploy/set inputs of the contracts appropriately, there will be no issue. Additionally admins are trusted as noted in READ.ME they can break certain assumption of the code based on their actions, and

Support

FAQs

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