RebateFi Hook

First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Submission Details
Impact: low
Likelihood: low
Invalid

Missing constructor sanity checks can permanently misconfigure the hook

Description

  • Normally, a constructor that wires a hook to a specific PoolManager and token should validate that both addresses are non-zero and sane. This prevents a single deployment misconfiguration from bricking the hook forever.

  • In this implementation, the constructor accepts _poolManager and _ReFi and assigns them without any validation. A deployment mistake (wrong address or zero address) will create a hook instance that cannot be fixed because ReFi is immutable and the PoolManager address is baked into BaseHook.

/// @notice Initializes the hook with pool manager and ReFi token address
/// @param _poolManager Address of the Uniswap V4 pool manager
/// @param _ReFi Address of the ReFi token
@> constructor(IPoolManager _poolManager, address _ReFi)
@> BaseHook(_poolManager)
@> Ownable(msg.sender)
@> {
@> ReFi = _ReFi;
@> }

Risk

Likelihood:

  • When deployment is orchestrated via scripts or environment variables, _ReFi or _poolManager can be left uninitialized, mis-typed, or copied from the wrong network, resulting in an incorrect or zero address.

  • When the same tooling is reused across testnets and mainnet, there is a realistic chance that outdated addresses are reused without being updated, especially in early iterations.

Impact:

  • If _ReFi is deployed as address(0) or a wrong token address, the hook will never treat the intended ReFi token as special, breaking fee logic and potentially affecting all hooked pools; this cannot be corrected on-chain and requires redeploying a new hook plus pool migration.

  • If _poolManager is incorrect, the hook is effectively not integrated with the real Uniswap V4 PoolManager, causing initialization and swaps to misbehave or fail, again forcing a redeploy and migration.

Proof of Concept

contract MisconfiguredDeployment {
IPoolManager public poolManager; // correctly set to the real PoolManager
address public refiToken; // BUG: accidentally left as address(0)
function deployBrokenHook() external returns (address) {
// refiToken == address(0) due to a deployment script bug
ReFiSwapRebateHook hook = new ReFiSwapRebateHook(poolManager, refiToken);
// At this point:
// - hook.ReFi() == address(0)
// - Any pools involving the real ReFi token will not match hook.ReFi
// - The hook logic relying on the ReFi address is effectively non-functional
//
// There is no way to change ReFi later because it is immutable.
return address(hook);
}
}

The only way to fix this is:

  1. Deploy a new hook with correct constructor arguments.

  2. Migrate pools / integrations to the new hook address.

Recommended Mitigation

Add simple require checks in the constructor to fail-fast on misconfiguration.

- constructor(IPoolManager _poolManager, address _ReFi)
- BaseHook(_poolManager)
- Ownable(msg.sender)
- {
- ReFi = _ReFi;
- }
+ constructor(IPoolManager _poolManager, address _ReFi)
+ BaseHook(_poolManager)
+ Ownable(msg.sender)
+ {
+ require(address(_poolManager) != address(0), "ReFiHook: invalid poolManager");
+ require(_ReFi != address(0), "ReFiHook: invalid ReFi token");
+ ReFi = _ReFi;
+ }

You can further harden this pattern by:

  • Passing an explicit owner address into the constructor, so deployment scripts can immediately set ownership to a multisig/governance contract.

  • Asserting _poolManager matches the expected canonical PoolManager for the deployment chain, if you know that address in advance.

Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!