Skip to content

Tee audit fixes #226

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

Open
roger-bai-coinbase wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Tee audit fixes #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7ee57f6
remove NotImplemented error (finding 2)
roger-bai-coinbase Mar 25, 2026
567a3a0
remove verifier/aggregator ID sets and EnumerableSet (findings 3 and 11)
roger-bai-coinbase Mar 25, 2026
e86119a
remove NotImplemented error (finding 2)
roger-bai-coinbase Mar 25, 2026
d6a9be5
remove verifier/aggregator ID sets and EnumerableSet (findings 3 and 11)
roger-bai-coinbase Mar 25, 2026
eca4ce7
Merge branch 'tee-audit-fixes' of https://github.com/base/contracts i…
roger-bai-coinbase Mar 25, 2026
f88f8d0
hardcode the keccak256 length (finding 5)
roger-bai-coinbase Mar 25, 2026
7acbf62
Add comment on allowing MAX_AGE old attestations (Finding 7)
roger-bai-coinbase Mar 25, 2026
4719a37
Prevent FROZEN address from being added as a verifier (Finding 8)
roger-bai-coinbase Mar 25, 2026
37edf92
Add stricter time checks (Findings 9 and 10)
roger-bai-coinbase Mar 25, 2026
2f95cc0
fix tests
roger-bai-coinbase Mar 25, 2026
dfe5571
semver lock
roger-bai-coinbase Mar 25, 2026
c7589c7
semver lock on clean build
roger-bai-coinbase Mar 25, 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
60 changes: 0 additions & 60 deletions interfaces/multiproof/tee/INitroEnclaveVerifier.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,42 +165,6 @@ interface INitroEnclaveVerifier {
*/
function getZkConfig(ZkCoProcessorType _zkCoProcessor) external view returns (ZkCoProcessorConfig memory);

/**
* @dev Returns all supported verifier program IDs for a coprocessor
* @param _zkCoProcessor Type of ZK coprocessor
* @return Array of all supported verifier program IDs
*/
function getVerifierIds(ZkCoProcessorType _zkCoProcessor) external view returns (bytes32[] memory);

/**
* @dev Returns all supported aggregator program IDs for a coprocessor
* @param _zkCoProcessor Type of ZK coprocessor
* @return Array of all supported aggregator program IDs
*/
function getAggregatorIds(ZkCoProcessorType _zkCoProcessor) external view returns (bytes32[] memory);

/**
* @dev Checks if a verifier program ID is in the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _verifierId Verifier program ID to check
* @return True if the ID is supported
*/
function isVerifierIdSupported(ZkCoProcessorType _zkCoProcessor, bytes32 _verifierId) external view returns (bool);

/**
* @dev Checks if an aggregator program ID is in the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _aggregatorId Aggregator program ID to check
* @return True if the ID is supported
*/
function isAggregatorIdSupported(
ZkCoProcessorType _zkCoProcessor,
bytes32 _aggregatorId
)
external
view
returns (bool);

/**
* @dev Gets the verifier address for a specific route
* @param _zkCoProcessor Type of ZK coprocessor
Expand Down Expand Up @@ -317,30 +281,6 @@ interface INitroEnclaveVerifier {
*/
function updateAggregatorId(ZkCoProcessorType _zkCoProcessor, bytes32 _newAggregatorId) external;

/**
* @dev Removes a verifier program ID from the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _verifierId Verifier program ID to remove
*
* Requirements:
* - Only callable by contract owner
* - Cannot remove the currently active (latest) verifier ID
* - ID must exist in the supported set
*/
function removeVerifierId(ZkCoProcessorType _zkCoProcessor, bytes32 _verifierId) external;

/**
* @dev Removes an aggregator program ID from the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _aggregatorId Aggregator program ID to remove
*
* Requirements:
* - Only callable by contract owner
* - Cannot remove the currently active (latest) aggregator ID
* - ID must exist in the supported set
*/
function removeAggregatorId(ZkCoProcessorType _zkCoProcessor, bytes32 _aggregatorId) external;

/**
* @dev Adds a route-specific verifier override
* @param _zkCoProcessor Type of ZK coprocessor
Expand Down
8 changes: 4 additions & 4 deletions snapshots/semver-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,12 @@
"sourceCodeHash": "0xfa0464c07c06fddc98ba20e9a362ba10ecf94496556d0f7ac88d1986f79a8a6b"
},
"src/multiproof/tee/TEEProverRegistry.sol:TEEProverRegistry": {
"initCodeHash": "0x4c89ecad0d48b6da64ef7f489326aae63b7fbcd33f4fed949a496afd1be49009",
"sourceCodeHash": "0x71a4016022b8a15f5ceddf2d597700046fcbe9ce4dbc7d7f2174d68606b3c614"
"initCodeHash": "0x95bf7d37bd7218dc48a176d443599af7dffcb83b57ed63226bd84a3facd8d5f2",
"sourceCodeHash": "0x4435163589299d9b0bc42a80258ebb3eee12ef94e6f1bd439d144b6628b366c8"
},
"src/multiproof/tee/TEEProverRegistry.sol:TEEProverRegistry:dispute": {
"initCodeHash": "0xfd56342b83d37499a848961ec12f982cee40297a4c59c5d448ed5a78795d3751",
"sourceCodeHash": "0x71a4016022b8a15f5ceddf2d597700046fcbe9ce4dbc7d7f2174d68606b3c614"
"initCodeHash": "0x3a50409059ad8313df38cd76747defa6ae976dcc8ca37b8333c78ef85c7661cd",
"sourceCodeHash": "0x4435163589299d9b0bc42a80258ebb3eee12ef94e6f1bd439d144b6628b366c8"
},
"src/multiproof/tee/TEEVerifier.sol:TEEVerifier": {
"initCodeHash": "0xeab3964cb2e6bbf3b660a8dedcd01286b74894bf76cf979d0b415fc7ca140ade",
Expand Down
117 changes: 5 additions & 112 deletions src/multiproof/tee/NitroEnclaveVerifier.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.0;

import { Ownable } from "@solady/auth/Ownable.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import {
INitroEnclaveVerifier,
ZkCoProcessorType,
Expand Down Expand Up @@ -43,8 +42,6 @@ import { ISP1Verifier } from "lib/sp1-contracts/contracts/src/ISP1Verifier.sol";
* - Timestamp validation prevents replay attacks within the configured time window
*/
contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
using EnumerableSet for EnumerableSet.Bytes32Set;

/// @dev Sentinel address to indicate a route has been permanently frozen
address private constant FROZEN = address(0xdead);

Expand All @@ -63,12 +60,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
/// @dev Hash of the trusted AWS Nitro Enclave root certificate
bytes32 public rootCert;

/// @dev Set of all supported verifier program IDs per coprocessor
mapping(ZkCoProcessorType => EnumerableSet.Bytes32Set) private _verifierIdSet;

/// @dev Set of all supported aggregator program IDs per coprocessor
mapping(ZkCoProcessorType => EnumerableSet.Bytes32Set) private _aggregatorIdSet;

/// @dev Route-specific verifier overrides (selector -> verifier address)
mapping(ZkCoProcessorType => mapping(bytes4 selector => address zkVerifier)) private _zkVerifierRoutes;

Expand All @@ -80,9 +71,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
/// @dev Error thrown when an unsupported or unknown ZK coprocessor type is used
error Unknown_Zk_Coprocessor();

/// @dev Error thrown when attempting to remove the currently active (latest) program ID
error CannotRemoveLatestProgramId(ZkCoProcessorType zkCoProcessor, bytes32 identifier);

/// @dev Error thrown when a ZK route has been permanently frozen
error ZkRouteFrozen(ZkCoProcessorType zkCoProcessor, bytes4 selector);

Expand All @@ -101,9 +89,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
/// @dev Thrown when attempting to set a program ID that is already the latest
error ProgramIdAlreadyLatest(ZkCoProcessorType zkCoProcessor, bytes32 identifier);

/// @dev Thrown when attempting to remove or operate on a program ID that does not exist in the set
error ProgramIdNotFound(ZkCoProcessorType zkCoProcessor, bytes32 identifier);

/// @dev Thrown when a zero address is provided where a verifier address is required
error ZeroVerifierAddress();

Expand All @@ -116,12 +101,12 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
/// @dev Thrown when the first certificate in a chain does not match the stored root certificate
error RootCertMismatch(bytes32 expected, bytes32 actual);

/// @dev Thrown when calling verifyWithProgramId or batchVerifyWithProgramId, which are intentionally disabled
error NotImplemented();

/// @dev Error thrown when a zero maxTimeDiff is provided
error ZeroMaxTimeDiff();

/// @dev Thrown when a zero address is provided for the verifier
error InvalidVerifierAddress();

// ============ Events ============

/// @dev Emitted when a new verifier program ID is added/updated
Expand All @@ -130,9 +115,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
/// @dev Emitted when a new aggregator program ID is added/updated
event AggregatorIdUpdated(ZkCoProcessorType indexed zkCoProcessor, bytes32 indexed newId);

/// @dev Emitted when a program ID is removed from the supported set
event ProgramIdRemoved(ZkCoProcessorType indexed zkCoProcessor, bytes32 indexed programId, bool isAggregator);

/// @dev Emitted when a route-specific verifier is added
event ZkRouteAdded(ZkCoProcessorType indexed zkCoProcessor, bytes4 indexed selector, address verifier);

Expand Down Expand Up @@ -203,51 +185,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
return zkConfig[_zkCoProcessor];
}

/**
* @dev Returns all supported verifier program IDs for a coprocessor
* @param _zkCoProcessor Type of ZK coprocessor
* @return Array of all supported verifier program IDs
*/
function getVerifierIds(ZkCoProcessorType _zkCoProcessor) external view returns (bytes32[] memory) {
return _verifierIdSet[_zkCoProcessor].values();
}

/**
* @dev Returns all supported aggregator program IDs for a coprocessor
* @param _zkCoProcessor Type of ZK coprocessor
* @return Array of all supported aggregator program IDs
*/
function getAggregatorIds(ZkCoProcessorType _zkCoProcessor) external view returns (bytes32[] memory) {
return _aggregatorIdSet[_zkCoProcessor].values();
}

/**
* @dev Checks if a verifier program ID is in the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _verifierId Verifier program ID to check
* @return True if the ID is supported
*/
function isVerifierIdSupported(ZkCoProcessorType _zkCoProcessor, bytes32 _verifierId) external view returns (bool) {
return _verifierIdSet[_zkCoProcessor].contains(_verifierId);
}

/**
* @dev Checks if an aggregator program ID is in the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _aggregatorId Aggregator program ID to check
* @return True if the ID is supported
*/
function isAggregatorIdSupported(
ZkCoProcessorType _zkCoProcessor,
bytes32 _aggregatorId
)
external
view
returns (bool)
{
return _aggregatorIdSet[_zkCoProcessor].contains(_aggregatorId);
}

/**
* @dev Gets the verifier address for a specific route
* @param _zkCoProcessor Type of ZK coprocessor
Expand Down Expand Up @@ -409,7 +346,6 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
}

zkConfig[_zkCoProcessor].verifierId = _newVerifierId;
_verifierIdSet[_zkCoProcessor].add(_newVerifierId);
_verifierProofIds[_zkCoProcessor][_newVerifierId] = _newVerifierProofId;

emit VerifierIdUpdated(_zkCoProcessor, _newVerifierId, _newVerifierProofId);
Expand All @@ -427,50 +363,10 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
}

zkConfig[_zkCoProcessor].aggregatorId = _newAggregatorId;
_aggregatorIdSet[_zkCoProcessor].add(_newAggregatorId);

emit AggregatorIdUpdated(_zkCoProcessor, _newAggregatorId);
}

/**
* @dev Removes a verifier program ID from the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _verifierId Verifier program ID to remove
*/
function removeVerifierId(ZkCoProcessorType _zkCoProcessor, bytes32 _verifierId) external onlyOwner {
if (!_verifierIdSet[_zkCoProcessor].contains(_verifierId)) {
revert ProgramIdNotFound(_zkCoProcessor, _verifierId);
}

// Cannot remove the latest verifier ID - must update to a new one first
if (zkConfig[_zkCoProcessor].verifierId == _verifierId) {
revert CannotRemoveLatestProgramId(_zkCoProcessor, _verifierId);
}

_verifierIdSet[_zkCoProcessor].remove(_verifierId);
delete _verifierProofIds[_zkCoProcessor][_verifierId];
emit ProgramIdRemoved(_zkCoProcessor, _verifierId, false);
}

/**
* @dev Removes an aggregator program ID from the supported set
* @param _zkCoProcessor Type of ZK coprocessor
* @param _aggregatorId Aggregator program ID to remove
*/
function removeAggregatorId(ZkCoProcessorType _zkCoProcessor, bytes32 _aggregatorId) external onlyOwner {
if (!_aggregatorIdSet[_zkCoProcessor].contains(_aggregatorId)) {
revert ProgramIdNotFound(_zkCoProcessor, _aggregatorId);
}

// Cannot remove the latest aggregator ID - must update to a new one first
if (zkConfig[_zkCoProcessor].aggregatorId == _aggregatorId) {
revert CannotRemoveLatestProgramId(_zkCoProcessor, _aggregatorId);
}

_aggregatorIdSet[_zkCoProcessor].remove(_aggregatorId);
emit ProgramIdRemoved(_zkCoProcessor, _aggregatorId, true);
}

/**
* @dev Adds a route-specific verifier override
* @param _zkCoProcessor Type of ZK coprocessor
Expand All @@ -479,6 +375,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
*/
function addVerifyRoute(ZkCoProcessorType _zkCoProcessor, bytes4 _selector, address _verifier) external onlyOwner {
if (_verifier == address(0)) revert ZeroVerifierAddress();
if (_verifier == FROZEN) revert InvalidVerifierAddress();

if (_zkVerifierRoutes[_zkCoProcessor][_selector] == FROZEN) {
revert ZkRouteFrozen(_zkCoProcessor, _selector);
Expand Down Expand Up @@ -623,12 +520,8 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {

// Auto-add program IDs to the version sets and store verifierProofId mapping
if (_config.verifierId != bytes32(0)) {
_verifierIdSet[_zkCoProcessor].add(_config.verifierId);
_verifierProofIds[_zkCoProcessor][_config.verifierId] = _verifierProofId;
}
if (_config.aggregatorId != bytes32(0)) {
_aggregatorIdSet[_zkCoProcessor].add(_config.aggregatorId);
}
emit ZKConfigurationUpdated(_zkCoProcessor, _config, _verifierProofId);
}

Expand Down Expand Up @@ -687,7 +580,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
}
}
uint64 timestamp = journal.timestamp / 1000;
if (timestamp + maxTimeDiff < block.timestamp || timestamp > block.timestamp) {
if (timestamp + maxTimeDiff <= block.timestamp || timestamp >= block.timestamp) {
journal.result = VerificationResult.InvalidTimestamp;
return journal;
}
Expand Down
5 changes: 4 additions & 1 deletion src/multiproof/tee/TEEProverRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ contract TEEProverRegistry is OwnableManagedUpgradeable, ISemver {

if (journal.result != VerificationResult.Success) revert AttestationVerificationFailed();

// We allow attestations up to MAX_AGE old. This means a cert may be expired between when
// the attestation is generated and when it is submitted to this contract.
if (journal.timestamp / MS_PER_SECOND + MAX_AGE <= block.timestamp) revert AttestationTooOld();

// Validate PCR0 against the current AggregateVerifier's TEE_IMAGE_HASH
Expand All @@ -155,7 +157,8 @@ contract TEEProverRegistry is OwnableManagedUpgradeable, ISemver {
if (pubKey.length != 65) revert InvalidPublicKey();
bytes32 publicKeyHash;
assembly {
publicKeyHash := keccak256(add(pubKey, 0x21), sub(mload(pubKey), 1))
// Length is hardcoded to 64 to skip the 0x04 prefix and hash only the x and y coordinates
publicKeyHash := keccak256(add(pubKey, 0x21), 64)
}
address enclaveAddress = address(uint160(uint256(publicKeyHash)));

Expand Down
Loading
Loading