Tadle

Tadle
DeFi
30,000 USDC
View results
Submission Details
Severity: high
Valid

`TokenManager` Can Be Drained

Summary

The TokenManager fails to update internal accounting when processing withdrawals, permitting a caller to repeatedly double-spend their outstanding withdrawal balance.

Vulnerability Details

Throughout the codebase there are multiple calls to TokenManager's addTokenBalance(TokenBalanceType,address,address,uint256) function, which is used to credit an account their deposited funds.

As an example, we can see that funds credited to an untrusted _msgSender(), when a referral bonus is accrued.

This is to emphasize that it is trivial to create a token balance for an arbitrary (i.e. malicious third party) account address.

This token balance can be withdrawn from the TokenManager via the withdraw(address,TokenBalanceType) function, documented below for posterity:

/**
* @notice Withdraw
* @dev Caller must be owner
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
/// @audit verify_caller_has_withdraw_balance
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType]; /// @audit this_value_is_never_decreased_upon_withdraw
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
/// @audit withdraw_the_token
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( /// @audit transfers_tokens_does_not_update_internal_accounting
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( /// @audit transfers_tokens_does_not_update_internal_accounting
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

Notice that when claimAbleAmount is nonzero for the _msgSender() (i.e. an attacker), they are permitted to withdraw the claimAbleAmount from the contract, but this amount is never reduced.

This means an attacker may repeatedly invoke withdraw(address,TokenBalanceType) to withdraw the same amount continuously until the point of contract insolvency.

Impact

All funds in the protocol can be stolen from the TokenManager.

An attacker need initially instantiate a non-zero balance owed to their address in any unit of tokens currently held by the TokenManager (exploiting free access to one of the many available invocations to addTokenBalance(TokenBalanceType,address,address,uint256)) in order to repeatedly withdraw that amount until insolvency.

Tools Used

Manual Review

Recommendations

Ensure the withdrawn amount is zeroed out upon withdraw:

uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
/// @notice Mark the funds as withdrawn to prevent further withdrawal attempts.
userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-withdraw-userTokenBalanceMap-not-reset

Valid critical severity finding, the lack of clearance of the `userTokenBalanceMap` mapping allows complete draining of the CapitalPool contract. Note: This would require the approval issues highlighted in other issues to be fixed first (i.e. wrong approval address within `_transfer` and lack of approvals within `_safe_transfer_from` during ERC20 withdrawals)

Support

FAQs

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