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

Withdrawal Function Fails for Wrapped Native Tokens Due to failed approval

MinhTriet

Vulnerability Details

The TokenManager::withdraw function fails for wrapped native token because somehow CapitalPool::approve function fails.

When you try to withdraw using wrapped native token with [TokenManager::withdraw function](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137-L140)

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {

then it will call [TokenManager::_transfer function](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L160-L166)

​
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);

then it will call [CapitalPool::approve function](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L247)

ICapitalPool(_capitalPoolAddr).approve(address(this));

and [revert ApproveFailed()](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L36-L38).

if (!success) {
revert ApproveFailed();
}

Impact

  1. Users are unable to withdraw their non-native tokens.

  2. Core contract functionality is broken.

  3. Potential loss of user trust and funds lockup.

PoC code

function test_withdraw_native_token() public {
​
// set up code so there's balance for user to withdraw
vm.startPrank(user);
preMarktes.createOffer{value: 0.01 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.006175 * 1e18}(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskTaker(stock1Addr, 500);
vm.stopPrank();
​
​
// Check actual balance to withdraw > 0 and withdraw
vm.startPrank(user);
uint balanceToWithdraw = tokenManager.userTokenBalanceMap(
address(user),
address(weth9),
TokenBalanceType.RemainingCash
);
assertGt(balanceToWithdraw, 0);
console.log("user's token balance they can withdraw: ", balanceToWithdraw);
vm.expectRevert();
// Withdraw fail due to ApproveFailed()
tokenManager.withdraw(address(weth9), TokenBalanceType.RemainingCash);
​
vm.stopPrank();
}

Tools Used

Foundry

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-approve-wrong-address-input

If we consider the correct permissioned implementation for the `approve()` function within `CapitalPool.sol`, this would be a critical severity issue, because the withdrawal of funds will be permanently blocked and must be rescued by the admin via the `Rescuable.sol` contract, given it will always revert [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L36-L38) when attempting to call a non-existent function selector `approve` within the TokenManager contract. The argument up in the air is since the approval function `approve` was made permisionless, the `if` block within the internal `_transfer()` function will never be invoked if somebody beforehand calls approval for the TokenManager for the required token, so the transfer will infact not revert when a withdrawal is invoked. I will leave open for escalation discussions, but based on my first point, I believe high severity is appropriate.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.