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

Deposits Possible When Contract is Paused, Bypassing Intended Security Measure

salem

Summary

The current implementation allows users to deposit funds (tillIn) and create new markets even when the contract is paused. This behavior contradicts the documentation, which states that pausing the contract should halt all market activities. This inconsistency creates a risk where users can inadvertently lock their funds in the contract during a paused state and cannot withdraw them, while new markets can still be created, leading to further complications.

The Readme:

Maker
- Create buy offer
- Create sell offer
- Cancel your offer
- Abort your offer
​
Taker
- Place taker orders
- Relist stocks as new offers
​
Sell Offer Maker
- Deliver tokens during settlement
​
General User
- Fetch balances info
- Withdraw funds from your balances
​
Admin (Trust)
- Create a marketplace
- Take a marketplace offline
- Initialize system parameters, like WETH contract address, referral commission rate, etc.
- Set up collateral token list, like ETH, USDC, LINK, ankrETH, etc.
- Set `TGE` parameters for settlement, like token contract address, TGE time, etc.
- Grant privileges for users’ commission rates
- Pause all the markets @audit ---> doesn't pause all market as users can still deposut into markets and createMarkets
​

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L56

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L137

Vulnerability Details

The root cause of this vulnerability is the inconsistent application of pause functionality across different operations in the contract. Specifically:

  • The withdraw function in TokenManager has a whenNotPaused modifier.

  • The tillIn function in TokenManager and the functions that call it (createOffer and createTaker in PreMarkets) do not have any pause checks.

function createOffer(CreateOfferParams calldata params) external payable {
// ... No pause check
tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);
// ...
}
​
function createTaker(address _offer, uint256 _points) external payable {
// ... No Pause Check
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
// ...
}
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
// ...
}
​
function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
) external payable {
// ... (no pause check)
}

Impact

The primary impact of this vulnerability is that it undermines the entire purpose of the pause mechanism. The pause feature is designed to halt all critical contract interactions in case of emergencies or during maintenance. By allowing deposits when the contract is paused:

  1. It bypasses a critical security measure, potentially exacerbating any issues that led to the pause.

  2. It could lead to funds being deposited into a compromised or malfunctioning system.

  3. It creates an inconsistent contract state where some operations are allowed while others are not.

Tools Used

Manual Review

Recommendations

  • Add the whenNotPaused modifier to the tillIn function in TokenManager

  • Also, add the whenNotPaused modifier to createOffer and createTaker in PreMarkets for an additional layer of security

Updates

Lead Judging Commences

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

[invalid] finding-Rescuable-pause-no-effect

I believe this is informational and non-acceptable severity because: - A single pause on withdraw to be sufficient to pause the markets during times of emergencies, given that is the only function where collateral/point tokens/native ETH can be pulled from market transactions. - Every tadle market place can be switched offline by the admin via [`updateMarketPlaceStatus`](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/SystemConfig.sol#L160-L171) and is checked in market actions via [`checkMarketPlaceStatus`](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/MarketPlaceLibraries.sol#L54-L67) to be online. This prevents many major market actions including the creation, listing and settlement of offers.

Support

FAQs

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

Give us feedback!