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 about 1 month 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!