diff --git a/contracts/ILCPClientErrors.sol b/contracts/ILCPClientErrors.sol index 3bb1a68..86c2253 100644 --- a/contracts/ILCPClientErrors.sol +++ b/contracts/ILCPClientErrors.sol @@ -59,6 +59,8 @@ interface ILCPClientErrors { error LCPClientUpdateOperatorsPermissionless(); error LCPClientUpdateOperatorsSignatureUnexpectedOperator(address actual, address expected); + error LCPClientBaseInvalidConstructorParams(); + error LCPClientZKDCAPInvalidConstructorParams(); error LCPClientZKDCAPOutputNotValid(); error LCPClientZKDCAPUnrecognizedTCBStatus(); @@ -66,7 +68,7 @@ interface ILCPClientErrors { error LCPClientZKDCAPInvalidNextTcbEvaluationDataNumberInfo(); error LCPClientZKDCAPInvalidVerifierInfos(); error LCPClientZKDCAPInvalidVerifierInfoLength(); - error LCPClientZKDCAPInvalidVerifierInfoZKVMType(); + error LCPClientZKDCAPInvalidVerifierInfoRisc0Header(); error LCPClientZKDCAPUnsupportedZKVMType(); error LCPClientZKDCAPRisc0ImageIdNotSet(); error LCPClientZKDCAPUnexpectedIntelRootCAHash(); diff --git a/contracts/LCPClientBase.sol b/contracts/LCPClientBase.sol index 6eccd40..14abdf9 100644 --- a/contracts/LCPClientBase.sol +++ b/contracts/LCPClientBase.sol @@ -55,11 +55,17 @@ abstract contract LCPClientBase is ILightClient, ILCPClientErrors { /// @dev clientId => client storage mapping(string => ClientStorage) internal clientStorages; + /// @dev Reserved storage space to allow for layout changes in the future + uint256[50] private __gap; + // --------------------- Constructor --------------------- /// @custom:oz-upgrades-unsafe-allow constructor /// @param ibcHandler_ the address of the IBC handler contract constructor(address ibcHandler_) { + if (ibcHandler_ == address(0)) { + revert LCPClientBaseInvalidConstructorParams(); + } ibcHandler = ibcHandler_; } diff --git a/contracts/LCPClientIASOwnableUpgradeable.sol b/contracts/LCPClientIASOwnableUpgradeable.sol index 5fd92de..a718250 100644 --- a/contracts/LCPClientIASOwnableUpgradeable.sol +++ b/contracts/LCPClientIASOwnableUpgradeable.sol @@ -8,7 +8,9 @@ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Own /// @custom:oz-upgrades-unsafe-allow external-library-linking contract LCPClientIASOwnableUpgradeable is LCPClientIASBase, UUPSUpgradeable, OwnableUpgradeable { /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address ibcHandler, bool developmentMode) LCPClientIASBase(ibcHandler, developmentMode) {} + constructor(address ibcHandler, bool developmentMode) LCPClientIASBase(ibcHandler, developmentMode) { + _disableInitializers(); + } function initialize(bytes memory rootCACert) public initializer { initializeRootCACert(rootCACert); diff --git a/contracts/LCPClientZKDCAPBase.sol b/contracts/LCPClientZKDCAPBase.sol index 20f3cb0..9949251 100644 --- a/contracts/LCPClientZKDCAPBase.sol +++ b/contracts/LCPClientZKDCAPBase.sol @@ -264,6 +264,7 @@ abstract contract LCPClientZKDCAPBase is LCPClientBase { // The format is as follows: // - First byte (0): zkVM type identifier. // - Remaining bytes (1–N): zkVM-specific data. + // - N must be greater than or equal to 32. // // Currently, only RISC Zero zkVM (type=1) is supported, with the following format: // @@ -273,12 +274,19 @@ abstract contract LCPClientZKDCAPBase is LCPClientBase { // | 1–31 | Reserved (set as zero) | // | 32–63 | Image ID | uint256 vlen = verifierInfo.length; - if (vlen == 0) { + if (vlen < 32) { revert LCPClientZKDCAPInvalidVerifierInfoLength(); } - // Currently, the client only supports RISC Zero zkVM - if (uint8(verifierInfo[0]) != ZKVM_TYPE_RISC_ZERO) { - revert LCPClientZKDCAPInvalidVerifierInfoZKVMType(); + // Currently, the client only supports RISC Zero zkVM. + // Load the first 32 bytes of 'verifierInfo' into 'header'. + // According to the format, the first byte should be 0x01 (RISC Zero type), + // and the remaining 31 bytes must be zero (reserved field). + bytes32 header; + assembly { + header := mload(add(verifierInfo, 32)) + } + if (header != bytes32(bytes1(ZKVM_TYPE_RISC_ZERO))) { + revert LCPClientZKDCAPInvalidVerifierInfoRisc0Header(); } // risc0 verifier info should be 64 bytes if (vlen != 64) { diff --git a/contracts/LCPClientZKDCAPOwnableUpgradeable.sol b/contracts/LCPClientZKDCAPOwnableUpgradeable.sol index a97c80e..0e68823 100644 --- a/contracts/LCPClientZKDCAPOwnableUpgradeable.sol +++ b/contracts/LCPClientZKDCAPOwnableUpgradeable.sol @@ -10,7 +10,9 @@ contract LCPClientZKDCAPOwnableUpgradeable is LCPClientZKDCAPBase, UUPSUpgradeab /// @custom:oz-upgrades-unsafe-allow constructor constructor(address ibcHandler_, bool developmentMode_, bytes memory intelRootCA, address riscZeroVerifier_) LCPClientZKDCAPBase(ibcHandler_, developmentMode_, intelRootCA, riscZeroVerifier_) - {} + { + _disableInitializers(); + } function initialize() public initializer { __UUPSUpgradeable_init(); diff --git a/test/LCPClientZKDCAPTest.t.sol b/test/LCPClientZKDCAPTest.t.sol index 4f88ec7..06cba27 100644 --- a/test/LCPClientZKDCAPTest.t.sol +++ b/test/LCPClientZKDCAPTest.t.sol @@ -376,6 +376,43 @@ contract LCPClientZKDCAPTest is BasicTest { lc.zkDCAPRegisterEnclaveKey(clientId, msgData); } + function testRegisterEnclaveKeyInvalidRisc0Header() public { + string memory clientId = "lcp-zkdcap"; + TestLCPClientZKDCAPExtended lc = new TestLCPClientZKDCAPExtended( + address(this), false, ZKDCAPTestHelper.dummyIntelRootCACert(), address(new NopRiscZeroVerifier()) + ); + { + IbcLightclientsLcpV1ClientState.Data memory clientState = defaultClientState(); + clientState.zkdcap_verifier_infos[0][0] = 0x00; + bytes memory clientStateBytes = LCPProtoMarshaler.marshal(clientState); + bytes memory consensusStateBytes = LCPProtoMarshaler.marshal(defaultConsensusState()); + vm.expectRevert( + abi.encodeWithSelector(ILCPClientErrors.LCPClientZKDCAPInvalidVerifierInfoRisc0Header.selector) + ); + lc.initializeClient(clientId, clientStateBytes, consensusStateBytes); + } + { + IbcLightclientsLcpV1ClientState.Data memory clientState = defaultClientState(); + clientState.zkdcap_verifier_infos[0][0] = 0x02; + bytes memory clientStateBytes = LCPProtoMarshaler.marshal(clientState); + bytes memory consensusStateBytes = LCPProtoMarshaler.marshal(defaultConsensusState()); + vm.expectRevert( + abi.encodeWithSelector(ILCPClientErrors.LCPClientZKDCAPInvalidVerifierInfoRisc0Header.selector) + ); + lc.initializeClient(clientId, clientStateBytes, consensusStateBytes); + } + { + IbcLightclientsLcpV1ClientState.Data memory clientState = defaultClientState(); + clientState.zkdcap_verifier_infos[0][1] = 0x01; + bytes memory clientStateBytes = LCPProtoMarshaler.marshal(clientState); + bytes memory consensusStateBytes = LCPProtoMarshaler.marshal(defaultConsensusState()); + vm.expectRevert( + abi.encodeWithSelector(ILCPClientErrors.LCPClientZKDCAPInvalidVerifierInfoRisc0Header.selector) + ); + lc.initializeClient(clientId, clientStateBytes, consensusStateBytes); + } + } + function testRegisterEnclaveKeyEnclaveDebugMismatch() public { string memory clientId = "lcp-zkdcap"; // developmentMode=false but output.enclaveDebugEnabled is set to true