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

`tillIn` function does not call `whenNotPaused` modifier though it should

rbserver

Summary

Since the tillIn function does not call the whenNotPaused modifier but the withdraw function does, more token amounts can be deposited but cannot be withdrawn when the token manager is paused. These newly deposited token amounts are locked in the capital pool as long as the token manager is paused.

Vulnerability Details

The following withdraw function calls the whenNotPaused modifier but the tillIn function below does not call the whenNotPaused modifier. When the token manager is paused, more token amounts can still be deposited into the capital pool since calling the tillIn function does not revert but these newly deposited token amounts cannot be withdrawn from the capital pool because calling the withdraw function reverts.

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

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
@> ) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
​
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
​
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
​
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
payable(msg.sender).transfer(claimAbleAmount);
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
...
}

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

function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
@> onlyRelatedContracts(tadleFactory, _msgSender())
@> onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
/// @notice return if amount is 0
if (_amount == 0) {
return;
}
​
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (capitalPoolAddr == address(0x0)) {
revert Errors.ContractIsNotDeployed();
}
​
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
/// @notice token is ERC20 token
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
...
}

Impact

Allowing more token amounts to be deposited through the tillIn function, which cannot be paused, can cause these newly deposited token amounts to be locked in the capital pool as long as the token manager is paused.

Tools Used

Manual Review

Recommended Mitigation

Similar to the withdraw function, the tillIn function can be updated to call the whenNotPaused modifier.

Updates

Lead Judging Commences

0xnevi Lead Judge about 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.