## Bug Description The SubstrateSignatureValidator contract contains a critical validation flaw in the `_updateAttestors` function where zero address checks are only performed on the first attestor, allowing `address(0)` to be inserted at any position beyond index 0. This creates a signature validation bypass that reduces the effective security threshold of the multi-signature scheme. The vulnerability exists in the `_updateAttestors` function: ```solidity function _updateAttestors(address[] memory attestors) internal { if (attestors.length == 0) revert EmptyAttestorsList(); if (attestors[0] == address(0)) revert InvalidAttestorAddress(); // Only checks first element if (attestors.length < _threshold) revert AttestorsLengthLessThanThreshold(); uint256 attestorsLength = attestors.length; for (uint256 i = 1; i < attestorsLength; ++i) { // solhint-disable-next-line gas-strict-inequalities if (attestors[i] <= attestors[i - 1]) revert InvalidAttestorList(); } _attestors = attestors; emit AttestorsUpdated(attestors); } ``` The critical flaw is that zero address validation only occurs for `attestors[0]`, while positions 1 through n-1 can contain `address(0)` without triggering the `InvalidAttestorAddress` revert. The subsequent sorting validation allows `address(0)` to pass through as long as it maintains the ascending order requirement. ### Impact: Multi-Signature Security Threshold Reduction When `address(0)` exists in the attestor list, attackers can exploit the `validateMessage` function to reduce the effective signature threshold by one. The validation logic treats invalid signatures that recover to `address(0)` as valid attestor matches: ```solidity function validateMessage(bytes32 message, bytes32[] calldata r, bytes32[] calldata s, uint8[] calldata v) external view returns (bool result) { // ... validation logic ... for (uint256 i = 0; i < signaturesLength; ++i) { address recoveredAddress = ecrecover(message, v[i], r[i], s[i]); // Can return address(0) for invalid sigs while (attestorIndex < attestorsLength && _attestors[attestorIndex] < recoveredAddress) { ++attestorIndex; } if (attestorIndex < attestorsLength && _attestors[attestorIndex] == recoveredAddress) { ++validSignaturesCount; // Increments for address(0) match! ++attestorIndex; } if (validSignaturesCount == _threshold) return true; } return false; } ``` When `ecrecover` fails due to invalid signature parameters, it returns `address(0)`. If `address(0)` exists as an attestor, this failure is treated as a valid signature match, allowing attackers to meet the threshold requirement with fewer legitimate signatures. ### Economic and Security Impact This vulnerability undermines the fundamental security assumptions of multi-signature validation systems used for: 1. **Cross-chain bridge operations**: Reduced attestor requirements can compromise asset security 2. **Governance decisions**: Lower signature thresholds can enable unauthorized proposal execution 3. **Access control systems**: Weakened authentication requirements for critical operations 4. **Financial transaction validation**: Reduced security for high-value operations The impact severity scales with the threshold configuration - systems with lower thresholds face proportionally higher risk from this one-signature reduction. ## Proof of Concept ```solidity // Scenario: 3 attestors with 2-of-3 threshold becomes effectively 1-of-2 // Attestors: [address(0), 0x1111...alice, 0x2222...bob] with threshold = 2 1. Owner calls updateAttestorsAndThreshold() with: attestors = [address(0), alice, bob] // address(0) passes validation at index 0 threshold = 2 2. Attacker wants to validate malicious message with only 1 legitimate signature: - Provides 1 valid signature from alice: r[0], s[0], v[0] - Provides 1 invalid signature that recovers to address(0): r[1]=0, s[1]=0, v[1]=0 3. validateMessage execution: - ecrecover(message, v[0], r[0], s[0]) = alice ✓ (valid) - ecrecover(message, v[1], r[1], s[1]) = address(0) ✓ (matches attestor[0]) - validSignaturesCount = 2, meets threshold - Returns true despite only 1 legitimate signature 4. Result: 2-of-3 multi-sig security reduced to 1-of-2 effectively ``` ## Recommendation Implement comprehensive zero address validation for all attestor positions: ```solidity function _updateAttestors(address[] memory attestors) internal { if (attestors.length == 0) revert EmptyAttestorsList(); if (attestors.length < _threshold) revert AttestorsLengthLessThanThreshold(); uint256 attestorsLength = attestors.length; // Validate all attestor addresses and ordering in single loop for (uint256 i = 0; i < attestorsLength; ++i) { if (attestors[i] == address(0)) revert InvalidAttestorAddress(); // Check ALL positions // Validate ascending order (skip for first element) if (i > 0 && attestors[i] <= attestors[i - 1]) { revert InvalidAttestorList(); } } _attestors = attestors; emit AttestorsUpdated(attestors); } ``` Additionally, consider implementing explicit validation in `validateMessage` to reject signatures that recover to `address(0)`: ```solidity address recoveredAddress = ecrecover(message, v[i], r[i], s[i]); if (recoveredAddress == address(0)) continue; // Skip invalid signature recoveries ``` This ensures the multi-signature threshold cannot be bypassed through invalid signature injection and maintains the intended security model.

Cheatc0d3

Public reviews