Skip to content

fix: namespace lane storage to prevent aliasing with the updater mapping - #5

Closed
quintuskilbourn wants to merge 2 commits into
mainfrom
fix/namespace-lane-storage
Closed

quintuskilbourn wants to merge 2 commits into
mainfrom
fix/namespace-lane-storage

Conversation

@quintuskilbourn

Copy link
Copy Markdown
Collaborator

Summary

The lane storage slot was derived as keccak256(abi.encode(target, laneIndex))
with an unbounded, caller-supplied laneIndex. The isUpdater authorization
mapping sits at storage slot 0, so isUpdater[target][updater] resolves to
keccak256(abi.encode(updater, keccak256(abi.encode(target, 0)))) — the same
shape as a lane slot for target = updater, laneIndex = keccak256(abi.encode(target, 0)).

A caller could pick a laneIndex whose lane write lands on an authorization
entry, granting itself updater rights for any target, or corrupting existing
authorizations.

Fix

Namespace the lane derivation:

bytes32 public constant LANE_STORAGE_NAMESPACE = keccak256("PrioUpdateRegistry.lane");
// ...
keccak256(abi.encode(LANE_STORAGE_NAMESPACE, target, laneIndex))

Lane preimages are now 96 bytes and can no longer coincide with the 64-byte
authorization-mapping preimage.

Based on the fix by @dvush.

🤖 Generated with Claude Code

quintuskilbourn and others added 2 commits September 15, 2026 11:17
Lane slot 0 was derived as keccak256(abi.encode(target, laneIndex)) with a
caller-supplied laneIndex. The updater authorization mapping isUpdater lives at
slot 0, so isUpdater[target][updater] resolves to
keccak256(abi.encode(updater, keccak256(abi.encode(target, 0)))) -- the same
shape as a lane slot for target=updater, laneIndex=keccak256(abi.encode(target,
0)). A caller could therefore pick a laneIndex whose lane write lands on an
authorization entry, granting itself updater rights for any target (or
corrupting existing authorizations).

Namespace the lane derivation with LANE_STORAGE_NAMESPACE so lane preimages are
96 bytes and can no longer coincide with the 64-byte authorization preimage.

Based on the fix by @dvush.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thomasandal

Copy link
Copy Markdown

This fix is already on `main` and deployed: `8701e8e` ("lane storage namespace, new deploy", 2026-09-16) applies the same `LANE_STORAGE_NAMESPACE` derivation, and the README now lists the new mainnet instance `0xda7afeed021eafc1c1af9c362de477dad0396b81`, with `0xda7afeed01fe625cf15d187a19f94b45f00b8c5f` moved to "Legacy".

So this PR is superseded (it also conflicts with `main` now, and its CI fails on the `GasBenchmark` warm-read snapshot that `8701e8e` regenerated). @quintuskilbourn OK to close?

On the JIT side we are moving `DeployJit` and jit-router to the new registry. The pAMM lanes stay on the legacy instance until each maker migrates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants