From ab56b9b6caf833ea6a9295d22309cd178a4f0a7c Mon Sep 17 00:00:00 2001
From: Harald Hoyer <harald@matterlabs.dev>
Date: Wed, 14 Feb 2024 13:48:16 +0100
Subject: [PATCH] fix(sgx-dcap-quoteverify-rs): don't `assert` in
 `tee_qv_get_collateral()`

use `TryFrom<&sgx_ql_qve_collateral_t>` instead of `From<sgx_ql_qve_collateral_t>`

* return an error, if something goes wrong, instead of panic
* don't take ownership of the memory

Signed-off-by: Harald Hoyer <harald@matterlabs.dev>
---
 .../sgx-dcap-quoteverify-rs/src/lib.rs        | 50 +++++++++++--------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src/lib.rs b/QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src/lib.rs
index e5f9f8f5..416bc89b 100644
--- a/QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src/lib.rs
+++ b/QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src/lib.rs
@@ -69,34 +69,38 @@ pub struct QuoteCollateral {
     pub qe_identity: Vec<c_char>,
 }
 
-impl From<sgx_ql_qve_collateral_t> for QuoteCollateral {
-    fn from(collateral: sgx_ql_qve_collateral_t) -> Self {
-        fn raw_ptr_to_vec(data: *mut c_char, len: u32) -> Vec<c_char> {
-            assert!(!data.is_null());
-            unsafe { slice::from_raw_parts(data, len as _) }.to_vec()
+impl TryFrom<&sgx_ql_qve_collateral_t> for QuoteCollateral {
+    type Error = ();
+
+    fn try_from(collateral: &sgx_ql_qve_collateral_t) -> Result<Self, Self::Error> {
+        fn raw_ptr_to_vec(data: *mut c_char, len: u32) -> Result<Vec<c_char>, ()> {
+            if data.is_null() {
+                return Err(());
+            }
+            Ok(unsafe { slice::from_raw_parts(data, len as _) }.to_vec())
         }
 
-        QuoteCollateral {
+        Ok(QuoteCollateral {
             major_version: unsafe { collateral.__bindgen_anon_1.__bindgen_anon_1.major_version },
             minor_version: unsafe { collateral.__bindgen_anon_1.__bindgen_anon_1.minor_version },
             tee_type: collateral.tee_type,
             pck_crl_issuer_chain: raw_ptr_to_vec(
                 collateral.pck_crl_issuer_chain,
                 collateral.pck_crl_issuer_chain_size,
-            ),
-            root_ca_crl: raw_ptr_to_vec(collateral.root_ca_crl, collateral.root_ca_crl_size),
-            pck_crl: raw_ptr_to_vec(collateral.pck_crl, collateral.pck_crl_size),
+            )?,
+            root_ca_crl: raw_ptr_to_vec(collateral.root_ca_crl, collateral.root_ca_crl_size)?,
+            pck_crl: raw_ptr_to_vec(collateral.pck_crl, collateral.pck_crl_size)?,
             tcb_info_issuer_chain: raw_ptr_to_vec(
                 collateral.tcb_info_issuer_chain,
                 collateral.tcb_info_issuer_chain_size,
-            ),
-            tcb_info: raw_ptr_to_vec(collateral.tcb_info, collateral.tcb_info_size),
+            )?,
+            tcb_info: raw_ptr_to_vec(collateral.tcb_info, collateral.tcb_info_size)?,
             qe_identity_issuer_chain: raw_ptr_to_vec(
                 collateral.qe_identity_issuer_chain,
                 collateral.qe_identity_issuer_chain_size,
-            ),
-            qe_identity: raw_ptr_to_vec(collateral.qe_identity, collateral.qe_identity_size),
-        }
+            )?,
+            qe_identity: raw_ptr_to_vec(collateral.qe_identity, collateral.qe_identity_size)?,
+        })
     }
 }
 
@@ -415,15 +419,17 @@ pub fn tee_qv_get_collateral(quote: &[u8]) -> Result<QuoteCollateral, quote3_err
         qvl_sys::tee_qv_get_collateral(quote.as_ptr(), quote.len() as u32, &mut buf, &mut buf_len)
     } {
         quote3_error_t::SGX_QL_SUCCESS => {
-            assert!(!buf.is_null());
-            assert!(buf_len > 0);
-            assert_eq!(
-                buf.align_offset(mem::align_of::<sgx_ql_qve_collateral_t>()),
-                0
-            );
+            if buf.is_null()
+                || buf_len == 0
+                || buf.align_offset(mem::align_of::<sgx_ql_qve_collateral_t>()) != 0
+            {
+                return Err(quote3_error_t::SGX_QL_NO_QUOTE_COLLATERAL_DATA);
+            }
 
-            let collateral =
-                QuoteCollateral::from(unsafe { *(buf as *const sgx_ql_qve_collateral_t) });
+            // SAFETY: buf is not null, buf_len is not zero, and buf is aligned.
+            let orig_collateral = &unsafe { *(buf as *const sgx_ql_qve_collateral_t) };
+            let collateral = QuoteCollateral::try_from(orig_collateral)
+                .map_err(|_| quote3_error_t::SGX_QL_NO_QUOTE_COLLATERAL_DATA)?;
 
             match unsafe { qvl_sys::tee_qv_free_collateral(buf) } {
                 quote3_error_t::SGX_QL_SUCCESS => Ok(collateral),