Submission Details
Severity:
Invalid Pool Validation Logic in `_beforeInitialize`
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
The validation function checks `currency1` twice instead of checking both `currency0` and `currency1`, allowing pools without the ReFi token to be initialized.
**Description**:
* The normal behavior expects the hook to validate that the ReFi token is present in the pool (either as currency0 or currency1) before allowing pool initialization.
* The specific issue is that the condition checks `currency1` against `ReFi` twice, never checking `currency0`, which means pools where ReFi is currency0 will incorrectly revert, and pools without ReFi at all might pass validation incorrectly.
**Root cause in the codebase**:
```solidity
// @> src/RebateFiHook.sol:122-126
function _beforeInitialize(address, PoolKey calldata key, uint160) internal view override returns (bytes4) {
if (Currency.unwrap(key.currency1) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
```
The condition `currency1 != ReFi && currency1 != ReFi` is always false (a value cannot be both equal and not equal to ReFi), so this check never reverts. Additionally, `currency0` is never checked.
Risk
Likelihood:
-
* This occurs during every pool initialization attempt
* The validation logic is fundamentally broken and will always fail to properly validate
Impact:
-
* Pools can be initialized without the ReFi token, breaking the core functionality of the hook
* The hook will apply incorrect fees or fail to identify buy/sell directions correctly
* Protocol integrity is compromised as the hook's assumptions about pool composition are violated
Proof of Concept
The validation logic is fundamentally broken. Here are three scenarios demonstrating the issue:
```solidity
// Scenario 1: ReFi is currency0 (should pass, but check is wrong)
PoolKey memory key;
key.currency0 = Currency.wrap(ReFi); // ReFi token address: 0x1234...
key.currency1 = Currency.wrap(otherToken); // Other token: 0x5678...
// Current broken check in _beforeInitialize():
if (Currency.unwrap(key.currency1) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
// Evaluation: (0x5678 != 0x1234) && (0x5678 != 0x1234) = true && true = true
// Result: Reverts incorrectly! Pool with ReFi as currency0 cannot initialize
// Scenario 2: ReFi is currency1 (happens to work by accident)
key.currency0 = Currency.wrap(otherToken); // 0x5678...
key.currency1 = Currency.wrap(ReFi); // 0x1234...
// Current check:
if (Currency.unwrap(key.currency1) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
// Evaluation: (0x1234 != 0x1234) && (0x1234 != 0x1234) = false && false = false
// Result: Doesn't revert, pool initializes (works by accident, but logic is wrong)
// Scenario 3: ReFi is NOT in pool (CRITICAL: should revert but doesn't)
key.currency0 = Currency.wrap(tokenA); // 0xAAAA...
key.currency1 = Currency.wrap(tokenB); // 0xBBBB...
// Current check:
if (Currency.unwrap(key.currency1) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
// Evaluation: (0xBBBB != 0x1234) && (0xBBBB != 0x1234) = true && true = true
// Result: Should revert but the condition is logically impossible to be true
// Actually: The condition checks the same thing twice, so it's always false
// Pool initializes WITHOUT ReFi token - CRITICAL BUG!
```
**Step-by-step execution demonstrating the critical bug:**
1. Attacker creates a pool with two arbitrary tokens (no ReFi)
2. Pool initialization calls hook's `_beforeInitialize()`
3. Validation checks: `currency1 != ReFi && currency1 != ReFi`
4. This condition can never be true (same value compared to itself)
5. Validation always passes, pool initializes without ReFi
6. Hook's `_isReFiBuy()` logic fails because ReFi is not in the pool
7. Fee application becomes unpredictable, breaking protocol functionality
Recommended Mitigation
```diff
// src/RebateFiHook.sol:122-126
function _beforeInitialize(address, PoolKey calldata key, uint160) internal view override returns (bytes4) {
- if (Currency.unwrap(key.currency1) != ReFi &&
- Currency.unwrap(key.currency1) != ReFi) {
+ if (Currency.unwrap(key.currency0) != ReFi &&
+ Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
```
Updates
Lead Judging Commences
chaossr 11 days ago
Submission Judgement Published Assigned finding tags:
Faulty pool check; only checks currency1 twice, omitting currency0.
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.