From 8c31cfd1ed14ae08b21f3a1268fad7eb37c2b702 Mon Sep 17 00:00:00 2001 From: kadenzipfel Date: Thu, 2 Jul 2026 11:11:11 -0700 Subject: [PATCH 1/2] Fix RolesAuthority storage collision --- src/auth/RolesAuthority.huff | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/auth/RolesAuthority.huff b/src/auth/RolesAuthority.huff index 3974ae05..9f88b2b6 100644 --- a/src/auth/RolesAuthority.huff +++ b/src/auth/RolesAuthority.huff @@ -47,7 +47,8 @@ /// @notice Helper to get the role for a capability #define macro GET_ROLE_FOR_CAPABILITY() = takes (2) returns (1) { - LOAD_ELEMENT_FROM_KEYS(0x00) // [role] + [GET_ROLES_WITH_CAPABILITY_LOCATION] // [location, target, sig] + LOAD_ELEMENT_FROM_KEYS_2D(0x00) // [role] } /// @notice Checks if a Role has a Capability @@ -67,7 +68,8 @@ /// @notice Helper to get if a capability is public #define macro IS_CAPABILITY_PUBLIC() = takes (2) returns (1) { - LOAD_ELEMENT_FROM_KEYS(0x00) // [public] + [IS_CAPABILITY_PUBLIC_LOCATION] // [location, target, sig] + LOAD_ELEMENT_FROM_KEYS_2D(0x00) // [public] } /// @notice Checks if an account can call a function on a given address @@ -115,7 +117,8 @@ 0x44 calldataload // [value] 0x24 calldataload // [sig, value] 0x04 calldataload // [target, sig, value] - STORE_ELEMENT_FROM_KEYS(0x00) // [] + [IS_CAPABILITY_PUBLIC_LOCATION] // [location, target, sig, value] + STORE_ELEMENT_FROM_KEYS_2D(0x00) // [] // Emit the capability updated event 0x44 calldataload // [value] @@ -158,7 +161,8 @@ // Store the new capability 0x44 calldataload // [sig, updated] 0x24 calldataload // [target, sig, updated] - STORE_ELEMENT_FROM_KEYS(0x00) // [] + [GET_ROLES_WITH_CAPABILITY_LOCATION] // [location, target, sig, updated] + STORE_ELEMENT_FROM_KEYS_2D(0x00) // [] // Jump to the emit log label emit_log jump @@ -177,7 +181,8 @@ // Store the new capability 0x44 calldataload // [sig, capabilies] 0x24 calldataload // [target, sig, capabilies] - STORE_ELEMENT_FROM_KEYS(0x00) // [] + [GET_ROLES_WITH_CAPABILITY_LOCATION] // [location, target, sig, capabilies] + STORE_ELEMENT_FROM_KEYS_2D(0x00) // [] // Emit the capability updated event emit_log: From 286468875a4c8897eb207f453ef7777c496bef4a Mon Sep 17 00:00:00 2001 From: kadenzipfel Date: Thu, 2 Jul 2026 11:11:37 -0700 Subject: [PATCH 2/2] Add test to validate collision bug fix --- test/auth/RolesAuthority.t.sol | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/auth/RolesAuthority.t.sol b/test/auth/RolesAuthority.t.sol index 3dbf60a8..7d5c6281 100644 --- a/test/auth/RolesAuthority.t.sol +++ b/test/auth/RolesAuthority.t.sol @@ -121,4 +121,31 @@ contract RolesAuthorityTest is Test, NonMatchingSelectorsHelper { assertEq(roleAuth.hasRole(user, role), true); } + + /// @notice Test capability maps do not collide + function testCapabilityMapsDoNotCollide() public { + address target = address(0xDEAD); + bytes4 sig = bytes4(0xdeadbeef); + address mallory = address(0xBAD); + + // Grant a specific role the capability. It is NOT made public. + vm.prank(OWNER); + roleAuth.setRoleCapability(5, target, sig, true); + assertTrue(roleAuth.doesRoleHaveCapability(5, target, sig)); + + // A role bitmap must not be misread as "public": Mallory holds no roles... + assertFalse(roleAuth.hasRole(mallory, 5)); + // ...so Mallory must not be authorized to call the capability. + assertFalse(roleAuth.canCall(mallory, target, sig)); + + // A user actually granted the role should be authorized. + vm.prank(OWNER); + roleAuth.setUserRole(mallory, 5, true); + assertTrue(roleAuth.canCall(mallory, target, sig)); + + // Toggling the distinct public-capability flag must leave the role bitmap intact. + vm.prank(OWNER); + roleAuth.setPublicCapability(target, sig, false); + assertTrue(roleAuth.doesRoleHaveCapability(5, target, sig)); + } }