Refactoring enclave-sample aligning sgx BKM.

lum1n0us · lum1n0us · commit 288abe3941f1 · 2026-02-05T11:34:42.000+08:00
- Buffer interface redesign to separate input and output
- prevent untrusted pointer
diff --git a/product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.config.xml b/product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.config.xml
@@ -8,7 +8,8 @@
   <ReservedMemExecutable>1</ReservedMemExecutable>
   <TCSNum>10</TCSNum>
   <TCSPolicy>1</TCSPolicy>
-  <DisableDebug>0</DisableDebug>
+  <!-- SECURITY FIX: Debug mode disabled for production enclave security -->
+  <DisableDebug>1</DisableDebug>
   <MiscSelect>0</MiscSelect>
   <MiscMask>0xFFFFFFFF</MiscMask>
 </EnclaveConfiguration>
diff --git a/product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.cpp b/product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.cpp
@@ -81,6 +81,140 @@ static EnclaveModule *enclave_module_list = NULL;
 static korp_mutex enclave_module_list_lock = OS_THREAD_MUTEX_INITIALIZER;
 #endif
 
+/* SECURITY FIX: Handle table for secure EnclaveModule reference management */
+#define MAX_MODULES 128
+typedef struct HandleTableEntry {
+    uint32 id;
+    EnclaveModule *module_ref;
+    bool in_use;
+} HandleTableEntry;
+
+static HandleTableEntry module_table[MAX_MODULES] = { 0 };
+static uint32 next_module_id = 1;
+static korp_mutex module_table_lock = OS_THREAD_MUTEX_INITIALIZER;
+
+/* SECURITY: Allocate secure handle for EnclaveModule, preventing pointer
+ * exposure */
+static uint32
+allocate_module_handle(EnclaveModule *module)
+{
+    uint32 handle_id = 0;
+
+    os_mutex_lock(&module_table_lock);
+
+    /* Find free slot */
+    for (uint32 i = 0; i < MAX_MODULES; i++) {
+        if (!module_table[i].in_use) {
+            module_table[i].id = next_module_id++;
+            module_table[i].module_ref = module;
+            module_table[i].in_use = true;
+            handle_id = module_table[i].id;
+            break;
+        }
+    }
+
+    os_mutex_unlock(&module_table_lock);
+
+    if (handle_id == 0) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written,
+                    "SECURITY WARNING: Module handle table full\n");
+    }
+
+    return handle_id;
+}
+
+/* SECURITY: Lookup EnclaveModule by handle ID, preventing direct pointer access
+ */
+static EnclaveModule *
+lookup_module_by_handle(uint32 handle_id)
+{
+    EnclaveModule *module = NULL;
+
+    if (handle_id == 0)
+        return NULL;
+
+    os_mutex_lock(&module_table_lock);
+
+    for (uint32 i = 0; i < MAX_MODULES; i++) {
+        if (module_table[i].in_use && module_table[i].id == handle_id) {
+            module = module_table[i].module_ref;
+            break;
+        }
+    }
+
+    os_mutex_unlock(&module_table_lock);
+
+    return module;
+}
+
+/* SECURITY FIX: Handle table for secure wasm_module_inst_t reference management
+ */
+#define MAX_INSTANCES 128
+typedef struct InstanceTableEntry {
+    uint32 id;
+    wasm_module_inst_t inst_ref;
+    bool in_use;
+} InstanceTableEntry;
+
+static InstanceTableEntry instance_table[MAX_INSTANCES] = { 0 };
+static uint32 next_instance_id = 1;
+static korp_mutex instance_table_lock = OS_THREAD_MUTEX_INITIALIZER;
+
+/* SECURITY: Allocate secure handle for wasm_module_inst_t, preventing pointer
+ * exposure */
+static uint32
+allocate_instance_handle(wasm_module_inst_t inst)
+{
+    uint32 handle_id = 0;
+
+    os_mutex_lock(&instance_table_lock);
+
+    for (uint32 i = 0; i < MAX_INSTANCES; i++) {
+        if (!instance_table[i].in_use) {
+            instance_table[i].id = next_instance_id++;
+            instance_table[i].inst_ref = inst;
+            instance_table[i].in_use = true;
+            handle_id = instance_table[i].id;
+            break;
+        }
+    }
+
+    os_mutex_unlock(&instance_table_lock);
+
+    if (handle_id == 0) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written,
+                    "SECURITY WARNING: Instance handle table full\n");
+    }
+
+    return handle_id;
+}
+
+/* SECURITY: Lookup wasm_module_inst_t by handle ID, preventing direct pointer
+ * access */
+static wasm_module_inst_t
+lookup_instance_by_handle(uint32 handle_id)
+{
+    wasm_module_inst_t inst = NULL;
+
+    if (handle_id == 0)
+        return NULL;
+
+    os_mutex_lock(&instance_table_lock);
+
+    for (uint32 i = 0; i < MAX_INSTANCES; i++) {
+        if (instance_table[i].in_use && instance_table[i].id == handle_id) {
+            inst = instance_table[i].inst_ref;
+            break;
+        }
+    }
+
+    os_mutex_unlock(&instance_table_lock);
+
+    return inst;
+}
+
 #if WASM_ENABLE_GLOBAL_HEAP_POOL != 0
 static char global_heap_buf[WASM_GLOBAL_HEAP_SIZE] = { 0 };
 #endif
@@ -277,7 +411,18 @@ handle_cmd_load_module(uint64 *args, uint32 argc)
         return;
     }
 
-    *(EnclaveModule **)args_org = enclave_module;
+    /* SECURITY FIX: Return secure handle ID instead of direct pointer */
+    uint32 enclave_module_id = allocate_module_handle(enclave_module);
+    if (enclave_module_id == 0) {
+        /* Handle table full - cleanup and return error */
+        if (!enclave_module->is_xip_file)
+            wasm_runtime_free(enclave_module);
+        else
+            os_munmap(enclave_module, (uint32)total_size);
+        *(void **)args_org = NULL;
+        return;
+    }
+    *(uint32 *)args_org = enclave_module_id;
 
 #if WASM_ENABLE_LIB_RATS != 0
     /* Calculate the module hash */
@@ -368,7 +513,10 @@ static void
 handle_cmd_instantiate_module(uint64 *args, uint32 argc)
 {
     uint64 *args_org = args;
-    EnclaveModule *enclave_module = *(EnclaveModule **)args++;
+    /* SECURITY FIX: Use handle lookup instead of direct pointer from untrusted
+     * host */
+    uint32 module_handle_id = *(uint32 *)args++;
+    EnclaveModule *enclave_module = lookup_module_by_handle(module_handle_id);
     uint32 stack_size = *(uint32 *)args++;
     uint32 heap_size = *(uint32 *)args++;
     char *error_buf = *(char **)args++;
@@ -377,7 +525,7 @@ handle_cmd_instantiate_module(uint64 *args, uint32 argc)
 
     bh_assert(argc == 5);
 
-    if (!runtime_inited) {
+    if (!runtime_inited || !enclave_module) {
         *(void **)args_org = NULL;
         return;
     }
@@ -389,7 +537,14 @@ handle_cmd_instantiate_module(uint64 *args, uint32 argc)
         return;
     }
 
-    *(wasm_module_inst_t *)args_org = module_inst;
+    /* SECURITY FIX: Return secure handle ID instead of direct pointer */
+    uint32 instance_id = allocate_instance_handle(module_inst);
+    if (instance_id == 0) {
+        wasm_runtime_deinstantiate(module_inst);
+        *(void **)args_org = NULL;
+        return;
+    }
+    *(uint32 *)args_org = instance_id;
 
     LOG_VERBOSE("Instantiate module success.\n");
 }
@@ -515,7 +670,9 @@ static void
 handle_cmd_set_wasi_args(uint64 *args, int32 argc)
 {
     uint64 *args_org = args;
-    EnclaveModule *enclave_module = *(EnclaveModule **)args++;
+    /* SECURITY FIX: Use handle lookup instead of direct pointer from host */
+    uint32 module_handle_id = *(uint32 *)args++;
+    EnclaveModule *enclave_module = lookup_module_by_handle(module_handle_id);
     char **dir_list = *(char ***)args++;
     uint32 dir_list_size = *(uint32 *)args++;
     char **env_list = *(char ***)args++;
@@ -533,7 +690,48 @@ handle_cmd_set_wasi_args(uint64 *args, int32 argc)
 
     bh_assert(argc == 10);
 
-    if (!runtime_inited) {
+    if (!runtime_inited || !enclave_module) {
+        *args_org = false;
+        return;
+    }
+
+    /* SECURITY FIX: Validate all pointer arrays before use */
+    if (dir_list_size > 0
+        && (!dir_list
+            || !sgx_is_outside_enclave(dir_list,
+                                       sizeof(char *) * dir_list_size))) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written, "SECURITY ERROR: Invalid dir_list\n");
+        *args_org = false;
+        return;
+    }
+
+    if (env_list_size > 0
+        && (!env_list
+            || !sgx_is_outside_enclave(env_list,
+                                       sizeof(char *) * env_list_size))) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written, "SECURITY ERROR: Invalid env_list\n");
+        *args_org = false;
+        return;
+    }
+
+    if (wasi_argc > 0
+        && (!wasi_argv
+            || !sgx_is_outside_enclave(wasi_argv,
+                                       sizeof(char *) * wasi_argc))) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written, "SECURITY ERROR: Invalid wasi_argv\n");
+        *args_org = false;
+        return;
+    }
+
+    if (addr_pool_list_size > 0
+        && (!addr_pool_list
+            || !sgx_is_outside_enclave(addr_pool_list,
+                                       sizeof(char *) * addr_pool_list_size))) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written, "SECURITY ERROR: Invalid addr_pool_list\n");
         *args_org = false;
         return;
     }
@@ -695,6 +893,28 @@ void
 ecall_handle_command(unsigned cmd, unsigned char *cmd_buf,
                      unsigned cmd_buf_size)
 {
+    /* SECURITY FIX: Validate buffer before processing */
+    if (!cmd_buf || cmd_buf_size < sizeof(uint64)) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written,
+                    "SECURITY ERROR: Invalid buffer parameters\n");
+        return;
+    }
+
+    if (!sgx_is_outside_enclave(cmd_buf, cmd_buf_size)) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written,
+                    "SECURITY ERROR: Buffer not outside enclave\n");
+        return;
+    }
+
+    if (cmd_buf_size % sizeof(uint64) != 0) {
+        int bytes_written = 0;
+        ocall_print(&bytes_written,
+                    "SECURITY ERROR: Buffer alignment invalid\n");
+        return;
+    }
+
     uint64 *args = (uint64 *)cmd_buf;
     uint32 argc = cmd_buf_size / sizeof(uint64);
 
