Submission Details
Severity:
`TokenManager` contract fails to decrement the user's balance after a withdrawal operation.
## Summary
The `userTokenBalanceMap` in the `TokenManager` contract fails to decrement the user's balance after a withdrawal operation. This critical vulnerability allows users to withdraw their funds multiple times, potentially draining the contract of assets.
## Vulnerability Details
The vulnerability is located in the `TokenManager` contract, specifically in the interaction between the `addTokenBalance` and `withdraw` functions.
1. The `addTokenBalance` function correctly increases the user's balance:
```javascript
function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress,
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
userTokenBalanceMap[_accountAddress][_tokenAddress][_tokenBalanceType] += _amount;
emit AddTokenBalance(
_accountAddress,
_tokenAddress,
_tokenBalanceType,
_amount
);
}
```
2. However, the `withdraw` function fails to decrement the balance after a successful withdrawal:
```javascript
function withdraw(address _tokenAddress, TokenBalanceType _tokenBalanceType) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
// The balance is not decremented here
// userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimAbleAmount;
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (_tokenAddress == wrappedNativeToken) {
// ... (native token withdrawal logic)
} else {
// ... (ERC20 token withdrawal logic)
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
```
This oversight allows a user to call the withdraw function multiple times, each time withdrawing their full balance, as the userTokenBalanceMap is never updated to reflect previous withdrawals.
## Impact
The contract could be drained of assets as users exploit the ability to withdraw multiple times. This could lead to significant financial losses for the protocol and other users.
## Tools Used
Manual Review
## Recommendations
Please follow CEI to avoid reentrancy issues when fixing this bug
```diff
function withdraw(address _tokenAddress, TokenBalanceType _tokenBalanceType) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimAbleAmount;
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (_tokenAddress == wrappedNativeToken) {
// ... (native token withdrawal logic)
} else {
// ... (ERC20 token withdrawal logic)
}
// The balance is not decremented here
// userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimAbleAmount;
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
```
Updates
Lead Judging Commences
0xnevi 10 months ago
Submission Judgement Published 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)
Prize pool breakdown
Total prize
30,000 USDC
nSLOC
1,22924 USDC / LOC
25,000 USDC
2,750 USDC
2,250 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.