Skip to content

fix(safe): close transaction-shape and guard validation gaps [ENG-4235] #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
makemake-kbo merged 6 commits into from
Aug 5, 2026
Merged

fix(safe): close transaction-shape and guard validation gaps [ENG-4235] #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ebbc1d
fix(safe): close assertion and guard audit gaps
makemake-kbo Aug 3, 2026
de064df
fix(safe): preserve unlimited allowance policy
makemake-kbo Aug 3, 2026
69a56c0
fix(safe): bound batch approval validation cost
makemake-kbo Aug 4, 2026
e6ade7b
fix(safe): reject mixed batch allowance grants
makemake-kbo Aug 5, 2026
711df59
docs(safe): clarify batch-wide grant restriction
makemake-kbo Aug 5, 2026
2d3ee45
fix(safe): keep allowance accumulation stack bounded
makemake-kbo Aug 5, 2026
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
40 changes: 34 additions & 6 deletions examples/safe-guard/script/DeployCredibleSafeGuard.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ contract DeployCredibleSafeGuard is Script {
/// @notice Thrown when a required registry read (isCredibleBlock / lastCredibleBlock) does not
/// return a single, well-formed 32-byte word.
error RegistryReadFailed(address registry, string read);
/// @notice Thrown when the registry reports a block that cannot yet have been credible.
error RegistryLastCredibleBlockInFuture(address registry, uint256 reportedBlock, uint256 currentBlock);

uint256 internal constant REGISTRY_READ_GAS_LIMIT = 50_000;

function run() external returns (CredibleSafeGuard guard) {
address registry = vm.envAddress("CREDIBLE_REGISTRY");
Expand Down Expand Up @@ -56,14 +60,38 @@ contract DeployCredibleSafeGuard is Script {
// it passes the length check but the guard's runtime decode treats it as unreadable
// (see CredibleSafeGuard._tryIsCredibleBlock's `value > 1` branch), which would otherwise
// let a registry silently deploy a permanently-fail-open guard.
(bool credibleOk, bytes memory credibleData) =
registry.staticcall(abi.encodeCall(ICredibleRegistry.isCredibleBlock, (block.number)));
if (!credibleOk || credibleData.length != 32 || abi.decode(credibleData, (uint256)) > 1) {
(bool credibleOk, uint256 credibleWord) =
_boundedRegistryRead(registry, abi.encodeCall(ICredibleRegistry.isCredibleBlock, (block.number)));
if (!credibleOk || credibleWord > 1) {
revert RegistryReadFailed(registry, "isCredibleBlock");
}

(bool lastOk, bytes memory lastData) =
registry.staticcall(abi.encodeCall(ICredibleRegistry.lastCredibleBlock, ()));
if (!lastOk || lastData.length != 32) revert RegistryReadFailed(registry, "lastCredibleBlock");
(bool lastOk, uint256 lastCredibleBlock) =
_boundedRegistryRead(registry, abi.encodeCall(ICredibleRegistry.lastCredibleBlock, ()));
if (!lastOk) revert RegistryReadFailed(registry, "lastCredibleBlock");
if (lastCredibleBlock > block.number) {
revert RegistryLastCredibleBlockInFuture(registry, lastCredibleBlock, block.number);
}
}

/// @dev Mirrors the guard's runtime boundary: 50k gas, exactly one return word, and no
/// unbounded returndata allocation. Deployment rejects failures; runtime fails open.
function _boundedRegistryRead(address registry, bytes memory callData)
internal
view
returns (bool readable, uint256 value)
{
assembly ("memory-safe") {
readable := staticcall(
REGISTRY_READ_GAS_LIMIT,
registry,
add(callData, 0x20),
mload(callData),
0x00,
0x20
)
readable := and(readable, eq(returndatasize(), 0x20))
value := mload(0x00)
}
}
}
96 changes: 2 additions & 94 deletions examples/safe/src/SafeConfigLockAssertion.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,97 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import {PhEvm} from "credible-std/PhEvm.sol";
import {SafeConfigLockHelpers} from "./SafeConfigLockHelpers.sol";

/// @title SafeConfigLockAssertion
/// @author Phylax Systems
/// @notice Locks the critical configuration envelope for a Safe multisig.
/// @dev The assertion checks the Safe after each monitored transaction:
/// - threshold and owner count stay above configured minimums;
/// - owner and module sets match one of the approved set hashes;
/// - transaction guard, module guard, and fallback handler match expected addresses.
///
/// Address set hashes are computed by sorting addresses ascending and then hashing
/// `abi.encode(sortedAddresses)`. For modules, `bytes32(0)` in the approved hash list
/// is a sentinel meaning "modules must be disabled".
contract SafeConfigLockAssertion is SafeConfigLockHelpers {
uint256 public immutable minThreshold;
uint256 public immutable minOwners;
address public immutable expectedGuard;
address public immutable expectedModuleGuard;
address public immutable expectedFallbackHandler;

bytes32[] public approvedOwnerSetHashes;
bytes32[] public approvedModuleSetHashes;

constructor(
uint256 minThreshold_,
uint256 minOwners_,
bytes32[] memory approvedOwnerSetHashes_,
bytes32[] memory approvedModuleSetHashes_,
address expectedGuard_,
address expectedModuleGuard_,
address expectedFallbackHandler_
) {
require(approvedOwnerSetHashes_.length != 0, "SafeConfigLock: owner hashes empty");
require(approvedModuleSetHashes_.length != 0, "SafeConfigLock: module hashes empty");

minThreshold = minThreshold_;
minOwners = minOwners_;
expectedGuard = expectedGuard_;
expectedModuleGuard = expectedModuleGuard_;
expectedFallbackHandler = expectedFallbackHandler_;

for (uint256 i; i < approvedOwnerSetHashes_.length; ++i) {
approvedOwnerSetHashes.push(approvedOwnerSetHashes_[i]);
}

for (uint256 i; i < approvedModuleSetHashes_.length; ++i) {
approvedModuleSetHashes.push(approvedModuleSetHashes_[i]);
}

_registerReshiramSpec();
}

function triggers() external view override {
registerStorageChangeTrigger(this.assertSafeConfiguration.selector);
}

/// @notice Checks the Safe config after the triggering transaction has completed.
/// @dev Fails when a Safe transaction leaves owners, modules, guards, or fallback handling
/// outside the deployment-time policy. A zero module-set hash in the approved list
/// only approves the empty module set.
function assertSafeConfiguration() external view {
address safe = ph.getAssertionAdopter();
PhEvm.ForkId memory post = _postTx();

address[] memory owners = _ownersAt(safe, post);
uint256 threshold = _thresholdAt(safe, post);

require(threshold >= minThreshold, "SafeConfigLock: threshold below minimum");
require(owners.length >= minOwners, "SafeConfigLock: owner count below minimum");
require(
_isApprovedHash(hashAddressSet(owners), approvedOwnerSetHashes, false),
"SafeConfigLock: owner set not approved"
);

address[] memory modules = _modulesAt(safe, post);
require(
_isApprovedHash(hashAddressSet(modules), approvedModuleSetHashes, modules.length == 0),
"SafeConfigLock: module set not approved"
);

require(_guardAt(safe, post) == expectedGuard, "SafeConfigLock: guard mismatch");
require(_moduleGuardAt(safe, post) == expectedModuleGuard, "SafeConfigLock: module guard mismatch");
require(_fallbackHandlerAt(safe, post) == expectedFallbackHandler, "SafeConfigLock: fallback handler mismatch");
}

function approvedOwnerSetHashCount() external view returns (uint256) {
return approvedOwnerSetHashes.length;
}

function approvedModuleSetHashCount() external view returns (uint256) {
return approvedModuleSetHashes.length;
}
}
// Re-export the single maintained implementation for existing example imports.
import {SafeConfigLockAssertion} from "credible-std/protection/safe/SafeConfigLockAssertion.sol";
118 changes: 2 additions & 116 deletions examples/safe/src/SafeConfigLockHelpers.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,119 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import {Assertion} from "credible-std/Assertion.sol";
import {PhEvm} from "credible-std/PhEvm.sol";
import {AssertionSpec} from "credible-std/SpecRecorder.sol";

interface ISafeConfigLockTarget {
function getThreshold() external view returns (uint256);
function getOwners() external view returns (address[] memory);
function getModulesPaginated(address start, uint256 pageSize)
external
view
returns (address[] memory array, address next);
}

/// @title SafeConfigLockHelpers
/// @author Phylax Systems
/// @notice Shared constants and snapshot readers for Safe configuration assertions.
abstract contract SafeConfigLockHelpers is Assertion {
address internal constant SPEC_RECORDER = address(uint160(uint256(keccak256("SpecRecorder"))));
address internal constant SENTINEL_MODULES = address(0x1);

uint256 internal constant MODULE_PAGE_SIZE = 256;

bytes32 internal constant FALLBACK_HANDLER_STORAGE_SLOT =
0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5;
bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
bytes32 internal constant MODULE_GUARD_STORAGE_SLOT =
0xb104e0b93118902c651344349b610029d694cfdec91c589c91ebafbcd0289947;

/// @notice Computes the deterministic hash used by owner and module allow lists.
/// @dev Sorts the provided addresses in memory before hashing, so Safe linked-list order
/// does not affect the resulting set hash.
function hashAddressSet(address[] memory accounts) public pure returns (bytes32) {
_sortAddresses(accounts);
return keccak256(abi.encode(accounts));
}

function _ownersAt(address safe, PhEvm.ForkId memory fork) internal view returns (address[] memory owners) {
owners = abi.decode(_viewAt(safe, abi.encodeCall(ISafeConfigLockTarget.getOwners, ()), fork), (address[]));
}

function _thresholdAt(address safe, PhEvm.ForkId memory fork) internal view returns (uint256) {
return _readUintAt(safe, abi.encodeCall(ISafeConfigLockTarget.getThreshold, ()), fork);
}

function _modulesAt(address safe, PhEvm.ForkId memory fork) internal view returns (address[] memory modules) {
address next;
(modules, next) = abi.decode(
_viewAt(
safe,
abi.encodeCall(ISafeConfigLockTarget.getModulesPaginated, (SENTINEL_MODULES, MODULE_PAGE_SIZE)),
fork
),
(address[], address)
);
require(next == SENTINEL_MODULES, "SafeConfigLock: too many modules");
}

function _guardAt(address safe, PhEvm.ForkId memory fork) internal view returns (address) {
return _addressSlotAt(safe, GUARD_STORAGE_SLOT, fork);
}

function _moduleGuardAt(address safe, PhEvm.ForkId memory fork) internal view returns (address) {
return _addressSlotAt(safe, MODULE_GUARD_STORAGE_SLOT, fork);
}

function _fallbackHandlerAt(address safe, PhEvm.ForkId memory fork) internal view returns (address) {
return _addressSlotAt(safe, FALLBACK_HANDLER_STORAGE_SLOT, fork);
}

function _addressSlotAt(address safe, bytes32 slot, PhEvm.ForkId memory fork) internal view returns (address) {
return address(uint160(uint256(ph.loadStateAt(safe, slot, fork))));
}

function _isApprovedHash(bytes32 actualHash, bytes32[] storage approvedHashes, bool emptySet)
internal
view
returns (bool)
{
for (uint256 i; i < approvedHashes.length; ++i) {
if (approvedHashes[i] == actualHash) {
return true;
}

if (emptySet && approvedHashes[i] == bytes32(0)) {
return true;
}
}

return false;
}

function _sortAddresses(address[] memory accounts) internal pure {
for (uint256 i = 1; i < accounts.length; ++i) {
address current = accounts[i];
uint256 j = i;

while (j > 0 && uint160(accounts[j - 1]) > uint160(current)) {
accounts[j] = accounts[j - 1];
--j;
}

accounts[j] = current;
}
}

function _viewFailureMessage() internal pure override returns (string memory) {
return "SafeConfigLock: safe view failed";
}

function _registerReshiramSpec() internal {
(bool ok,) = SPEC_RECORDER.call(
abi.encodeWithSelector(bytes4(keccak256("registerAssertionSpec(uint8)")), AssertionSpec.Reshiram)
);
require(ok, "SafeConfigLock: spec registration failed");
}
}
// Re-export the single maintained helper symbol for existing example imports.
import {SafeConfigLockHelpers} from "credible-std/protection/safe/SafeConfigLockHelpers.sol";
Loading