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

Capital pool can be drained via reentrancy

audinarey

Summary

Due to missing state update in the TokenManager::withdraw(...)function, a user can drain the capital pool and steal funds.

Vulnerability Details

The TokenManager::withdraw(...)function can be called by eligible users who have claimable tokenBalanceType of a particular token.

However, the userTokenBalance[][][]is not updated for every withdrawal a user makes for a given tokenBalanceTypeof a particular _tokenAddressand as such a user can continue to call withdraw with the same parameters for the same tokenBalanceTypeand empty the capitalPoolAddr.

File: TokenManager.sol
139: function withdraw(
140: address _tokenAddress,
141: TokenBalanceType _tokenBalanceType
142: ) external whenNotPaused { // @audit it can be called directly
143: @> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
144: _tokenAddress
145: ][_tokenBalanceType];
146:
147: @> if (claimAbleAmount == 0) {
148: return;
149: }
.....
184:
185: emit Withdraw(
186: _msgSender(),
187: _tokenAddress,
188: _tokenBalanceType,
189: claimAbleAmount
190: );
191: }
​
​

Impact

Theft of users funds in the deposit pool due to reentrancy

Tools Used

Manual review

Recommendations

Update the TokenManager::withdraw(...)function as shown below

File: TokenManager.sol
139: function withdraw(
140: address _tokenAddress,
141: TokenBalanceType _tokenBalanceType
142: ) external whenNotPaused { // @audit it can be called directly
143: @> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
144: _tokenAddress
145: ][_tokenBalanceType];
146:
147: @> if (claimAbleAmount == 0) {
148: return;
149: }
+150: userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
.....
184:
185: emit Withdraw(
186: _msgSender(),
187: _tokenAddress,
188: _tokenBalanceType,
189: claimAbleAmount
190: );
191: }
​
​
Updates

Lead Judging Commences

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