Merge pull request #35 from datachainlab/audit-202503

Audit-202503

Signed-off-by: Jun Kimura <jun.kimura@datachain.jp>

Author: bluele

contracts/ILCPClientErrors.sol

@@ -59,14 +59,16 @@ interface ILCPClientErrors {
     error LCPClientUpdateOperatorsPermissionless();
     error LCPClientUpdateOperatorsSignatureUnexpectedOperator(address actual, address expected);
 
+    error LCPClientBaseInvalidConstructorParams();
+
     error LCPClientZKDCAPInvalidConstructorParams();
     error LCPClientZKDCAPOutputNotValid();
     error LCPClientZKDCAPUnrecognizedTCBStatus();
     error LCPClientZKDCAPCurrentTcbEvaluationDataNumberNotSet();
     error LCPClientZKDCAPInvalidNextTcbEvaluationDataNumberInfo();
     error LCPClientZKDCAPInvalidVerifierInfos();
     error LCPClientZKDCAPInvalidVerifierInfoLength();
-    error LCPClientZKDCAPInvalidVerifierInfoZKVMType();
+    error LCPClientZKDCAPInvalidVerifierInfoRisc0Header();
     error LCPClientZKDCAPUnsupportedZKVMType();
     error LCPClientZKDCAPRisc0ImageIdNotSet();
     error LCPClientZKDCAPUnexpectedIntelRootCAHash();

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_;
     }
 

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);

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) {

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();

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