Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,5 @@ scripts/*_output*.json
.env.devnet
.env.testnet
.env.mainnet

lib/
19 changes: 11 additions & 8 deletions hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@ loadAndValidateEnvironment();

const config: HardhatUserConfig = {
solidity: {
compilers: [{ version: '0.8.17' }],
settings: {
optimizer: {
enabled: true,
runs: 999999,
details: {
yul: true
compilers: [{
version: '0.8.17',
settings: {
viaIR: true,
Copy link
Author

Choose a reason for hiding this comment

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

@drinkcoffee I tweaked this settling to resolve a "stack too deep" error. Not sure if there's any unintended side effects that I'm not aware of. Pls advise.

Copy link
Author

Choose a reason for hiding this comment

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

@drinkcoffee I ended up changing this because it caused issues with the existing tests where revert messages were validated. I had to introduce an inline struct to reduce the number of variables on the stack.

optimizer: {
enabled: true,
runs: 999999,
details: {
yul: true
}
}
}
}
}]
},
paths: {
root: 'src',
Expand Down
12 changes: 6 additions & 6 deletions scripts/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ async function main(): Promise<EnvironmentInfo> {
// 4. Deploy startup wallet impl (PNR)
const startupWalletImpl = await deployContractViaCREATE2(env, wallets, 'StartupWalletImpl', [walletImplLocator.address]);

// --- Step 4: Deployed using CREATE2 Factory.
// 5. Deploy main module dynamic auth (CFC)
const mainModuleDynamicAuth = await deployContractViaCREATE2(env, wallets, 'MainModuleDynamicAuth', [factory.address, startupWalletImpl.address]);

// --- Step 5: Deployed using Passport Nonce Reserver.
// 6. Deploy immutable signer (PNR)
// --- Step 4: Deployed using Passport Nonce Reserver.
// 5. Deploy immutable signer (PNR)
const immutableSigner = await deployContractViaCREATE2(env, wallets, 'ImmutableSigner', [signerRootAdminPubKey, signerAdminPubKey, signerAddress]);

// --- Step 5: Deployed using CREATE2 Factory.
// 6. Deploy main module dynamic auth (CFC)
const mainModuleDynamicAuth = await deployContractViaCREATE2(env, wallets, 'MainModuleDynamicAuth', [factory.address, startupWalletImpl.address, immutableSigner.address]);

// --- Step 6: Deployed using alternate wallet (?)
// Fund the implementation changer
// WARNING: If the deployment fails at this step, DO NOT RERUN without commenting out the code a prior which deploys the contracts.
Expand Down
4 changes: 3 additions & 1 deletion scripts/step4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ async function step4(): Promise<EnvironmentInfo> {
const { network } = env;
const factoryAddress = '0x8Fa5088dF65855E0DaF87FA6591659893b24871d';
const startupWalletImplAddress = '0x8FD900677aabcbB368e0a27566cCd0C7435F1926';
const immutableSignerAddress = '0xcff469E561D9dCe5B1185CD2AC1Fa961F8fbDe61';

console.log(`[${network}] Starting deployment...`);
console.log(`[${network}] Factory address ${factoryAddress}`);
console.log(`[${network}] StartupWalletImpl address ${startupWalletImplAddress}`);
console.log(`[${network}] ImmutableSigner address ${immutableSignerAddress}`);

await waitForInput();

Expand All @@ -25,7 +27,7 @@ async function step4(): Promise<EnvironmentInfo> {

// --- Step 4: Deployed using CREATE2 Factory.
// Deploy main module dynamic auth (CFC)
const mainModuleDynamicAuth = await deployContractViaCREATE2(env, wallets, 'MainModuleDynamicAuth', [factoryAddress, startupWalletImplAddress]);
const mainModuleDynamicAuth = await deployContractViaCREATE2(env, wallets, 'MainModuleDynamicAuth', [factoryAddress, startupWalletImplAddress, immutableSignerAddress]);

fs.writeFileSync('step4.json', JSON.stringify({
factoryAddress: factoryAddress,
Expand Down
4 changes: 2 additions & 2 deletions src/contracts/mocks/MainModuleMockV1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "../modules/MainModuleDynamicAuth.sol";

contract MainModuleMockV1 is MainModuleDynamicAuth {
// solhint-disable-next-line no-empty-blocks
constructor(address _factory, address _startup) MainModuleDynamicAuth(_factory, _startup) {}
constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) MainModuleDynamicAuth(_factory, _startupWalletImpl, _immutableSignerContract) {}


}
}
2 changes: 1 addition & 1 deletion src/contracts/mocks/MainModuleMockV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "../modules/MainModuleDynamicAuth.sol";

contract MainModuleMockV2 is MainModuleDynamicAuth {
// solhint-disable-next-line no-empty-blocks
constructor(address _factory, address _startup) MainModuleDynamicAuth(_factory, _startup) {}
constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) MainModuleDynamicAuth(_factory, _startupWalletImpl, _immutableSignerContract) {}

function version() external pure override returns (uint256) {
return 2;
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/mocks/MainModuleMockV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "../modules/MainModuleDynamicAuth.sol";

contract MainModuleMockV3 is MainModuleDynamicAuth {
// solhint-disable-next-line no-empty-blocks
constructor(address _factory, address _startup) MainModuleDynamicAuth(_factory, _startup) {}
constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) MainModuleDynamicAuth(_factory, _startupWalletImpl, _immutableSignerContract) {}

function version() external pure override returns (uint256) {
return 3;
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/modules/MainModuleDynamicAuth.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract MainModuleDynamicAuth is
{

// solhint-disable-next-line no-empty-blocks
constructor(address _factory, address _startup) ModuleAuthDynamic (_factory, _startup) { }
constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) ModuleAuthDynamic (_factory, _startupWalletImpl, _immutableSignerContract) { }


/**
Expand Down
10 changes: 5 additions & 5 deletions src/contracts/modules/commons/ModuleAuth.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import "./ModuleERC165.sol";
abstract contract ModuleAuth is IModuleAuth, ModuleERC165, SignatureValidator, IERC1271Wallet {
using LibBytes for bytes;

uint256 private constant FLAG_SIGNATURE = 0;
uint256 private constant FLAG_ADDRESS = 1;
uint256 private constant FLAG_DYNAMIC_SIGNATURE = 2;
uint256 internal constant FLAG_SIGNATURE = 0;
uint256 internal constant FLAG_ADDRESS = 1;
uint256 internal constant FLAG_DYNAMIC_SIGNATURE = 2;

bytes4 private constant SELECTOR_ERC1271_BYTES_BYTES = 0x20c13b0b;
bytes4 private constant SELECTOR_ERC1271_BYTES32_BYTES = 0x1626ba7e;
Expand Down Expand Up @@ -49,7 +49,7 @@ abstract contract ModuleAuth is IModuleAuth, ModuleERC165, SignatureValidator, I
bytes32 _hash,
bytes memory _signature
)
internal override returns (bool)
internal virtual override returns (bool)
{
(bool verified, bool needsUpdate, bytes32 imageHash) = _signatureValidationWithUpdateCheck(_hash, _signature);
if (needsUpdate) {
Expand All @@ -74,7 +74,7 @@ abstract contract ModuleAuth is IModuleAuth, ModuleERC165, SignatureValidator, I
bytes32 _hash,
bytes memory _signature
)
internal view returns (bool, bool, bytes32)
internal view virtual returns (bool, bool, bytes32)
{
(
uint16 threshold, // required threshold signature
Expand Down
138 changes: 137 additions & 1 deletion src/contracts/modules/commons/ModuleAuthDynamic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,155 @@ pragma solidity 0.8.17;

import "./ModuleAuthUpgradable.sol";
import "./ImageHashKey.sol";
import "./ModuleStorage.sol";
import "./NonceKey.sol";
import "../../Wallet.sol";

import "../../utils/LibBytes.sol";

abstract contract ModuleAuthDynamic is ModuleAuthUpgradable {
using LibBytes for bytes;

bytes32 public immutable INIT_CODE_HASH;
address public immutable FACTORY;
address public immutable IMMUTABLE_SIGNER_CONTRACT;

constructor(address _factory, address _startupWalletImpl) {
constructor(address _factory, address _startupWalletImpl, address _immutableSignerContract) {
// Build init code hash of the deployed wallets using that module
bytes32 initCodeHash = keccak256(abi.encodePacked(Wallet.creationCode, uint256(uint160(_startupWalletImpl))));

INIT_CODE_HASH = initCodeHash;
FACTORY = _factory;
IMMUTABLE_SIGNER_CONTRACT = _immutableSignerContract;
}

/**
* @notice Verify if signer is default wallet owner
* @param _hash Hashed signed message
* @param _signature Array of signatures with signers ordered
* like the the keys in the multisig configs
*
* @dev The signature must be solidity packed and contain the total number of owners,
* the threshold, the weight and either the address or a signature for each owner.
*
* Each weight & (address or signature) pair is prefixed by a flag that signals if such pair
* contains an address or a signature. The aggregated weight of the signatures must surpass the threshold.
*
* Flag types:
* 0x00 - Signature
* 0x01 - Address
*
* E.g:
* abi.encodePacked(
* uint16 threshold,
* uint8 01, uint8 weight_1, address signer_1,
* uint8 00, uint8 weight_2, bytes signature_2,
* ...
* uint8 01, uint8 weight_5, address signer_5
* )
*/
function _signatureValidation(
bytes32 _hash,
bytes memory _signature
)
internal virtual override returns (bool)
{
(bool verified, bool needsUpdate, bytes32 imageHash) = _signatureValidationWithUpdateCheck(_hash, _signature);
if (needsUpdate) {
updateImageHashInternal(imageHash);
}
return verified;
}

/**
* @notice Verify signature and determine if image hash needs updating
* @param _hash Hashed signed message
* @param _signature Packed signature data containing threshold, flags, weights, and addresses/signatures
* @return verified True if the signature is valid and weight threshold is met
* @return needsUpdate True if the image hash needs to be stored (first transaction)
* @return imageHash The computed image hash from the signature
*
* @dev This function parses the signature, recovers/reads signer addresses, and validates them.
* For defensive validation, each extracted address is compared against IMMUTABLE_SIGNER_CONTRACT.
* If a match is found, it is recorded.
*
* Special case: If this is the first transaction (nonce == 0) and the immutable signer contract
* is one of the signers, the signature is automatically validated and approved without checking
* the stored image hash. This allows the immutable signer to bootstrap the wallet on first use.
*/
function _signatureValidationWithUpdateCheck(
bytes32 _hash,
bytes memory _signature
)
internal view override returns (bool, bool, bytes32)
{
(
uint16 threshold, // required threshold signature
uint256 rindex // read index
) = _signature.readFirstUint16();

// Start image hash generation
bytes32 imageHash = bytes32(uint256(threshold));

// Acumulated weight of signatures
uint256 totalWeight;

// Track if immutable signer contract is one of the signers
bool immutableSignerContractFound = false;

// Iterate until the image is completed
while (rindex < _signature.length) {
// Read next item type and addrWeight
uint256 flag; uint256 addrWeight; address addr;
(flag, addrWeight, rindex) = _signature.readUint8Uint8(rindex);

if (flag == FLAG_ADDRESS) {
// Read plain address
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we only use FLAG_SIGNATURE and FLAG_DYNAMIC_SIGNATURE. If I am correct, then we should remove the FLAG_ADDRESS option.

At attacker could create two valid signatures that are FLAG_SIGNATURE, and include the Immutable signer as a FLAG_ADDRESS. The checks below would deem the signature valid and bootstrap the wallet at line 109 and 120.

Copy link
Contributor

Choose a reason for hiding this comment

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

They don't need to create two valid signatures, as we don't check the weight. Hence, they could just create a _signature block that had Immutable signer as a FLAG_ADDRESS, and then they could set the image hash to a value of their choosing.

Copy link
Author

@naveen-imtb naveen-imtb Dec 11, 2025

Choose a reason for hiding this comment

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

You're right @drinkcoffee

This is a critical bug. I've updated the contract with the following changes:

  • Removed FLAG_ADDRESS support - This prevents attackers from including the immutable signer address in the signature without actually providing a valid signature from that address.
  • Required verified signatures from the Immutable signer contract - The immutable signer must now actually sign the transaction (via FLAG_SIGNATURE or FLAG_DYNAMIC_SIGNATURE) rather than just being listed as an address. The immutableSignerContractSigned flag is only set to true after the signature has been cryptographically verified.

These changes ensure that the bootstrap flow can only be triggered when the Immutable signer contract has genuinely signed the transaction, preventing unauthorized bootstrap attacks.

(addr, rindex) = _signature.readAddress(rindex);
} else if (flag == FLAG_SIGNATURE) {
// Read single signature and recover signer
bytes memory signature;
(signature, rindex) = _signature.readBytes66(rindex);
addr = recoverSigner(_hash, signature);

// Acumulate total weight of the signature
totalWeight += addrWeight;
} else if (flag == FLAG_DYNAMIC_SIGNATURE) {
// Read signer
(addr, rindex) = _signature.readAddress(rindex);

// Read signature size
uint256 size;
(size, rindex) = _signature.readUint16(rindex);

// Read dynamic size signature
bytes memory signature;
(signature, rindex) = _signature.readBytes(rindex, size);
require(isValidSignature(_hash, addr, signature), "ModuleAuthDynamic#_signatureValidation: INVALID_SIGNATURE");

// Acumulate total weight of the signature
totalWeight += addrWeight;
} else {
revert("ModuleAuthDynamic#_signatureValidation INVALID_FLAG");
}

// Defensive check: compare extracted address with target address
if (IMMUTABLE_SIGNER_CONTRACT != address(0) && addr == IMMUTABLE_SIGNER_CONTRACT) {
immutableSignerContractFound = true;
}

// Write weight and address to image
imageHash = keccak256(abi.encode(imageHash, addrWeight, addr));
}

// Check if this is the first transaction (nonce == 0) and immutable signer contract is one of the signers
uint256 currentNonce = uint256(ModuleStorage.readBytes32Map(NonceKey.NONCE_KEY, bytes32(uint256(0))));
if (currentNonce == 0 && immutableSignerContractFound) {
return (true, true, imageHash);
}

(bool verified, bool needsUpdate) = _isValidImage(imageHash);
return ((totalWeight >= threshold && verified), needsUpdate, imageHash);
}

/**
Expand Down
Loading