Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/MainnetController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { IERC20 } from "../lib/openzeppelin-contracts/contracts/token/ER
import { IERC20Metadata } from "../lib/openzeppelin-contracts/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import { IERC4626 } from "../lib/openzeppelin-contracts/contracts/interfaces/IERC4626.sol";

import { PoolKey } from "../lib/uniswap-v4-core/src/types/PoolKey.sol";

import { Ethereum } from "spark-address-registry/Ethereum.sol";

import { IALMProxy } from "./interfaces/IALMProxy.sol";
Expand Down Expand Up @@ -330,6 +332,10 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
{
_checkRole(DEFAULT_ADMIN_ROLE);

PoolKey memory poolKey = UniswapV4Lib.getPoolKeyFromPoolId(poolId);

require(address(poolKey.hooks) == address(0), "MC/pool-has-hooks");
Comment on lines +335 to +337
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Hook validation is incomplete - critical gap in swap operations.

While this validation correctly prevents configuring tick limits for pools with hooks, it doesn't prevent swaps on such pools. The swapUniswapV4 function (line 719-738) relies on maxSlippages[address(uint160(uint256(poolId)))] which is set via setMaxSlippage (line 267-274), bypassing this hook check entirely.

Attack scenario:

  1. Admin sets maxSlippage for a pool with hooks (no validation)
  2. Relayer calls swapUniswapV4 with that poolId
  3. Swap executes on a hooked pool, violating the "no hooks" requirement
🔎 Recommended fix: Add hook validation to swap operations

Option 1: Add validation in UniswapV4Lib.swap function

In src/libraries/UniswapV4Lib.sol, add hook validation after retrieving the poolKey:

 function swap(
     address proxy,
     address rateLimits,
     bytes32 poolId,
     address tokenIn,
     uint128 amountIn,
     uint128 amountOutMin,
     uint256 maxSlippage
 )
     external
 {
     require(maxSlippage != 0, "MC/max-slippage-not-set");

     PoolKey memory poolKey = getPoolKeyFromPoolId(poolId);
+
+    require(address(poolKey.hooks) == address(0), "MC/pool-has-hooks");

     require(
         tokenIn == Currency.unwrap(poolKey.currency0) ||

Option 2: Add validation in setMaxSlippage for Uniswap V4 pools

Modify setMaxSlippage to detect and validate Uniswap V4 poolIds. However, this is more complex as you'd need to distinguish pool types.

Recommendation: Option 1 is cleaner and ensures runtime protection.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/MainnetController.sol around lines 335-337 the code checks PoolKey memory
poolKey = UniswapV4Lib.getPoolKeyFromPoolId(poolId);
require(address(poolKey.hooks) == address(0), "MC/pool-has-hooks"); but swap
paths still allow swaps against pools with hooks via swapUniswapV4 (lines
~719-738) because maxSlippage is set elsewhere; to fix, add the same hooks
validation inside the UniswapV4Lib.swap implementation immediately after
retrieving the poolKey (or at the start of swapUniswapV4) so swaps revert for
hooked pools; use the same revert message/semantic ("MC/pool-has-hooks") and
ensure the check uses address(poolKey.hooks) == address(0) to block any pool
with nonzero hooks.


require(
((tickLowerMin == 0) && (tickUpperMax == 0) && (maxTickSpacing == 0)) ||
((maxTickSpacing > 0) && (tickLowerMin < tickUpperMax)),
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/UniswapV4Lib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ library UniswapV4Lib {
{
_checkTickLimits(tickLimits[poolId], tickLower, tickUpper);

PoolKey memory poolKey = _getPoolKeyFromPoolId(poolId);
PoolKey memory poolKey = getPoolKeyFromPoolId(poolId);

bytes memory callData = _getMintCalldata({
poolKey : poolKey,
Expand Down Expand Up @@ -176,7 +176,7 @@ library UniswapV4Lib {
{
require(maxSlippage != 0, "MC/max-slippage-not-set");

PoolKey memory poolKey = _getPoolKeyFromPoolId(poolId);
PoolKey memory poolKey = getPoolKeyFromPoolId(poolId);

require(
tokenIn == Currency.unwrap(poolKey.currency0) ||
Expand Down Expand Up @@ -243,6 +243,10 @@ library UniswapV4Lib {
_approveWithPermit2(proxy, tokenIn, _ROUTER, 0);
}

function getPoolKeyFromPoolId(bytes32 poolId) public view returns (PoolKey memory poolKey) {
return IPositionManagerLike(_POSITION_MANAGER).poolKeys(bytes25(poolId));
}

/**********************************************************************************************/
/*** Internal Interactive Functions ***/
/**********************************************************************************************/
Expand Down Expand Up @@ -528,10 +532,6 @@ library UniswapV4Lib {
return IPositionManagerLike(_POSITION_MANAGER).getPoolAndPositionInfo(tokenId);
}

function _getPoolKeyFromPoolId(bytes32 poolId) internal view returns (PoolKey memory poolKey) {
return IPositionManagerLike(_POSITION_MANAGER).poolKeys(bytes25(poolId));
}

function _getPoolKeyFromTokenId(uint256 tokenId)
internal view returns (PoolKey memory poolKey)
{
Expand Down
Loading