fix: namespace lane storage to prevent aliasing with the updater mapping - #5
Closed
quintuskilbourn wants to merge 2 commits into
Closed
quintuskilbourn wants to merge 2 commits into
quintuskilbourn wants to merge 2 commits into
Conversation
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>
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The lane storage slot was derived as
keccak256(abi.encode(target, laneIndex))with an unbounded, caller-supplied
laneIndex. TheisUpdaterauthorizationmapping sits at storage slot 0, so
isUpdater[target][updater]resolves tokeccak256(abi.encode(updater, keccak256(abi.encode(target, 0))))— the sameshape as a lane slot for
target = updater,laneIndex = keccak256(abi.encode(target, 0)).A caller could pick a
laneIndexwhose lane write lands on an authorizationentry, granting itself updater rights for any target, or corrupting existing
authorizations.
Fix
Namespace the lane derivation:
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