Submission Details
Impact:
Likelihood:
Missing Balance Check in `withdrawTokens`
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
The `withdrawTokens` function does not verify that the contract has sufficient balance before attempting to transfer, which will cause the transfer to revert but provides a poor user experience and could be exploited for griefing.
**Description**:
* The normal behavior expects the function to check if the contract has enough tokens before attempting a transfer, providing a clear error message if insufficient.
* The specific issue is that `withdrawTokens` directly calls `transfer` without checking the contract's balance first, so if the balance is insufficient, the call will revert with a generic ERC20 error rather than a clear, hook-specific error.
**Root cause in the codebase**:
```solidity
// @> src/RebateFiHook.sol:73-75
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token , amount);
}
```
No balance check is performed before the transfer.
Risk
Likelihood:
-
* This occurs when the owner attempts to withdraw more tokens than the contract holds
* Accidental over-withdrawal attempts or incorrect amount calculations can trigger this
Impact:
-
* Transaction reverts with unclear ERC20 error messages, making debugging difficult
* Owner must manually check balances before withdrawing
* Potential for griefing if owner repeatedly attempts invalid withdrawals (wastes gas)
Proof of Concept
The following demonstrates the poor user experience when attempting to withdraw more than available:
```solidity
// Scenario: Contract has accumulated 100 tokens from fees
address token = 0x1234...; // ReFi token
address owner = 0x5678...;
uint256 contractBalance = IERC20(token).balanceOf(address(rebateHook)); // 100 tokens
// Owner miscalculates or attempts to withdraw more than available
uint256 withdrawAmount = 200 tokens; // More than contract has
// Current implementation (no balance check):
rebateHook.withdrawTokens(token, owner, withdrawAmount);
// Execution flow:
// 1. Function called with amount = 200
// 2. No balance check performed
// 3. Direct call to IERC20(token).transfer(owner, 200)
// 4. ERC20 transfer checks: balanceOf(rebateHook) = 100 < 200
// 5. Transfer reverts with generic error (varies by ERC20 implementation):
// - OpenZeppelin: "ERC20: transfer amount exceeds balance"
// - Some tokens: Silent revert with no message
// 6. Transaction fails, gas consumed, no clear error from hook
// Problems:
// - Owner doesn't know why it failed (insufficient balance vs other issues)
// - Must manually query balance before each withdrawal attempt
// - Repeated failed attempts waste gas
// - No hook-specific error for better debugging
```
**Step-by-step execution:**
1. Owner calls `withdrawTokens(token, owner, 200)` when contract has 100 tokens
2. Function proceeds without checking balance
3. ERC20 `transfer()` is called with amount exceeding balance
4. Transfer reverts with generic ERC20 error message
5. Transaction fails, gas consumed, owner confused about failure reason
6. Owner must manually check balance and retry with correct amount
Recommended Mitigation
Add a balance check with a clear error message before attempting the transfer:
```diff
// src/RebateFiHook.sol:48-50
+ error InsufficientBalance(address token, uint256 requested, uint256 available);
// src/RebateFiHook.sol:73-75
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
+ uint256 balance = IERC20(token).balanceOf(address(this));
+ if (balance < amount) {
+ revert InsufficientBalance(token, amount, balance);
+ }
IERC20(token).transfer(to, amount);
- emit TokensWithdrawn(to, token , amount);
+ emit TokensWithdrawn(token, to, amount);
}
```
**Explanation:**
- Check contract balance before attempting transfer
- Revert with clear, hook-specific error if insufficient balance
- Error includes token address, requested amount, and available balance for debugging
- Prevents gas waste from failed transfers
- Improves user experience with informative error messages
- Follows best practice of validating inputs before external calls
Rewards breakdown
nSLOC
150This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.