Datachain LCP-zkDCAP

In the `check_tdx_module_tcb_status` function, a TDX module identity ID is generated using lowercase hexadecimal formatting: ```rust let tdx_module_identity_id = format!("TDX_{:02x}", tdx_module_version); ``` This identifier is then used in a direct string comparison when filtering module identities: ```rust .filter(|m| m.id == tdx_module_identity_id) ``` The reference C++ implementation in Intel's SGX-TDX-DCAP-QuoteVerificationLibrary explicitly normalizes string case before comparison: ```cpp std::string id = tdxModuleIdentity.getId(); std::transform(id.begin(), id.end(), id.begin(), ::toupper); return (id == tdxModuleIdentityId); ``` If a mismatch occurs due to case sensitivity: - The function will not find a matching TDX module identity - It will return `TdxModuleTcbValidationStatus::TcbNotSupported` - This will likely cause legitimate TDX attestations to fail verification While this is not a security vulnerability allowing unauthorized access, it is a correctness issue that could impact system functionality by causing false rejections of valid attestations.

The `validate_pck_cert()` function currently relies on the Common Name (CN) to match certificates within the chain. This approach introduces potential security risks, as the CN alone is neither unique nor sufficient to unambiguously identify a certificate. Matching solely on CN can result in ambiguity and open the door to certain attacks, such as an adversary creating a certificate with the same CN as a trusted one. According to [RFC 5280, section 4.1.2.4](https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.4), the Distinguished Name (DN) should be used instead: > "Certificate users MUST be prepared to process the issuer distinguished name and subject distinguished name (Section 4.1.2.6) fields to perform name chaining for certification path validation. Name chaining is performed by matching the issuer distinguished name in one certificate with the subject name in a CA certificate. Rules for comparing distinguished names are specified in Section 7.1." That said, we assess the risk to be low in this context, as the certificate must be signed by a trusted root certificate and SGX DCAP use cases are limited. As a result, it is unlikely that an attacker could successfully generate a PCK certificate with a colliding CN.

The logic used to validate the expiration of enclave keys in the `LCPClientZKDCAPBase.zkDCAPRegisterEnclaveKey()` function exhibits inconsistent behavior when `block.timestamp == output.validityNotAfter`. Specifically, the current implementation allows this edge case to pass when `key_expiration == 0`, but triggers a revert when `key_expiration != 0`. This inconsistency may lead to unexpected or confusing behavior for integrators or future auditors of the code. Although this is an edge case and would only cause a transient error, ensuring consistent validation logic is important for clarity and maintainability.

The current implementation of Intel SGX PCK certificate validation logic does not explicitly verify critical X.509 extensions such as `Key Usage` (OID: `2.5.29.15`) and `Basic Constraints` (OID: `2.5.29.19`). According to [RFC5280 section 4.2](https://datatracker.ietf.org/doc/html/rfc5280), a certificate-using system must reject a certificate if it encounters a critical extension it does not recognize or a critical extension that contains information it cannot process. According to the [Intel SGX PCK Certificate specification](https://api.trustedservices.intel.com/documents/Intel_SGX_PCK_Certificate_CRL_Spec-1.5.pdf), certificates have the following extensions marked as "critical": - **PCK Leaf Certificate**: `Key Usage` (`Digital Signature`, `Non Repudiation`), `Basic Constraints` (`CA:FALSE`). - **PCK Issuer CA Certificate**: `Key Usage` (`Certificate Sign`, `CRL Sign`), `Basic Constraints` (`CA:TRUE`, `pathlen:0`). - **Intel SGX Root CA Certificate**: `Key Usage` (`Certificate Sign`, `CRL Sign`), `Basic Constraints` (`CA:TRUE`, `pathlen:1`). - **TCB Singing Certificate**: `Key Usage` (`Digital Signature`, `Non Repudiation`), `Basic Constraints` (`CA:FALSE`). Failing to enforce these validations may allow certificates with inappropriate key usage or incorrect CA constraints, weakening the trustworthiness of the certificate chain.

In the `collateral.rs::from_bytes()` function, the sum of the lengths could potentially overflow. Specifically, these values are read from the input `slice`, meaning they can be manipulated. If an overflow occurs, the `QvCollateral` structure could be re-constructed from unexpected bytes. Here is the code snippet where the overflow risk can occur: ```rust if slice.len() < offset + tcb_info_json_len + qe_identity_json_len + sgx_intel_root_ca_der_len + sgx_tcb_signing_der_len + sgx_intel_root_ca_crl_der_len + sgx_pck_crl_der_len { bail!("Invalid QvCollateral length"); } ```

The `utils.rs::parse_x509_der_multi()` function contains a potential infinite loop vulnerability. When processing a sequence of X.509 certificates, the function iteratively calls `X509Certificate::from_der()` and updates the input slice `i` with the remaining bytes `j` returned by the parser. However, there is no check to ensure that the parser is making progress through the buffer. If the parser returns a non-empty slice (`j`) that is the same length as the input slice (`i`), the function will enter an infinite loop.

The upgradeable contracts `LCPClientIASOwnableUpgradeable` and `LCPClientZKDCAPOwnableUpgradeable` allow an attacker to initialize the implementation contract, which could lead to phishing and social engineering attacks. For more details, see the [OpenZeppelin guideline](https://docs.openzeppelin.com/upgrades-plugins/writing-upgradeable#initializing_the_implementation_contract).