Bladeren bron

[Pal/Linux-SGX] OCALL security hardening

  - Time-Of-Check-To-Time-Of-Use (TOCTOU) vulnerability is closed
    by first copying untrusted values inside enclave and then
    operating on these copied values; see sgx_copy_to_enclave().
  - Pointer/integer overflow is closed by comparing against a
    trusted maximum value in sgx_copy_to_enclave().
  - Untrusted stack overflows in sgx_alloc_on_ustack()/sgx_copy_to_ustack()
    are closed by checking for NULL return values.

These vulnerabilities were independently discovered and disclosed
by David Oswald, Jo van Bulck, and others.
Dmitrii Kuvaiskii 5 jaren geleden
bovenliggende
commit
40fc490fcd

+ 2 - 2
Pal/src/host/Linux-SGX/db_main.c

@@ -156,7 +156,7 @@ static const char** make_argv_list(void * uptr_src, uint64_t src_size) {
         return NULL;
     }
 
-    if (sgx_copy_to_enclave(data, uptr_src, src_size) != 0) {
+    if (!sgx_copy_to_enclave(data, src_size, uptr_src, src_size)) {
         goto free_and_err;
     }
     data[src_size - 1] = '\0';
@@ -211,7 +211,7 @@ void pal_linux_main(char * uptr_args, uint64_t args_size,
     int rv;
 
     struct pal_sec sec_info;
-    if (sgx_copy_to_enclave(&sec_info, uptr_sec_info, sizeof(sec_info)) != 0) {
+    if (!sgx_copy_to_enclave(&sec_info, sizeof(sec_info), uptr_sec_info, sizeof(sec_info))) {
         return;
     }
 

+ 42 - 17
Pal/src/host/Linux-SGX/enclave_framework.c

@@ -38,21 +38,8 @@ bool sgx_is_completely_outside_enclave(const void* addr, uint64_t size) {
     return enclave_base >= addr + size || enclave_top <= addr;
 }
 
-__attribute__((warn_unused_result))
-int sgx_copy_to_enclave(void * dst, void * uptr_src, uint64_t size) {
-    if (!sgx_is_completely_outside_enclave(uptr_src, size) ||
-        !sgx_is_completely_within_enclave(dst, size)) {
-        return -PAL_ERROR_DENIED;
-    }
-
-    memcpy(dst, uptr_src, size);
-
-    return 0;
-}
-
-void * sgx_ocalloc (uint64_t size)
-{
-    void * ustack = GET_ENCLAVE_TLS(ustack) - size;
+void* sgx_alloc_on_ustack(uint64_t size) {
+    void* ustack = GET_ENCLAVE_TLS(ustack) - size;
     if (!sgx_is_completely_outside_enclave(ustack, size)) {
         return NULL;
     }
@@ -60,11 +47,49 @@ void * sgx_ocalloc (uint64_t size)
     return ustack;
 }
 
-void sgx_ocfree (void)
-{
+void* sgx_copy_to_ustack(const void* ptr, uint64_t size) {
+    if (!sgx_is_completely_within_enclave(ptr, size)) {
+        return NULL;
+    }
+    void* uptr = sgx_alloc_on_ustack(size);
+    if (uptr) {
+        memcpy(uptr, ptr, size);
+    }
+    return uptr;
+}
+
+void sgx_reset_ustack(void) {
     SET_ENCLAVE_TLS(ustack, GET_ENCLAVE_TLS(ustack_top));
 }
 
+/* NOTE: Value from possibly untrusted uptr must be copied inside
+ * CPU register or enclave stack (to prevent TOCTOU). Function call
+ * achieves this. Attribute ensures no inline optimization. */
+__attribute__((noinline))
+bool sgx_copy_ptr_to_enclave(void** ptr, void* uptr, uint64_t size) {
+    assert(ptr);
+    if (!sgx_is_completely_outside_enclave(uptr, size)) {
+        *ptr = NULL;
+        return false;
+    }
+    *ptr = uptr;
+    return true;
+}
+
+/* NOTE: Value from possibly untrusted uptr and usize must be copied
+ * inside CPU registers or enclave stack (to prevent TOCTOU). Function
+ * call achieves this. Attribute ensures no inline optimization. */
+__attribute__((noinline))
+uint64_t sgx_copy_to_enclave(const void* ptr, uint64_t maxsize, const void* uptr, uint64_t usize) {
+    if (usize > maxsize ||
+        !sgx_is_completely_outside_enclave(uptr, usize) ||
+        !sgx_is_completely_within_enclave(ptr, usize)) {
+        return 0;
+    }
+    memcpy((void*) ptr, uptr, usize);
+    return usize;
+}
+
 int sgx_get_report (sgx_arch_hash_t * mrenclave,
                     sgx_arch_attributes_t * attributes,
                     void * enclave_data,

File diff suppressed because it is too large
+ 390 - 218
Pal/src/host/Linux-SGX/enclave_ocalls.c


+ 6 - 4
Pal/src/host/Linux-SGX/sgx_api.h

@@ -24,13 +24,15 @@
 
 int sgx_ocall (unsigned long code, void * ms);
 
-void * sgx_ocalloc (uint64_t size);
-void sgx_ocfree (void);
-
 bool sgx_is_completely_within_enclave (const void * addr, uint64_t size);
 bool sgx_is_completely_outside_enclave(const void * addr, uint64_t size);
 
-int sgx_copy_to_enclave(void * dst, void * uptr_src, uint64_t size);
+void* sgx_alloc_on_ustack(uint64_t size);
+void* sgx_copy_to_ustack(const void* ptr, uint64_t size);
+void sgx_reset_ustack(void);
+
+bool sgx_copy_ptr_to_enclave(void** ptr, void* uptr, uint64_t size);
+uint64_t sgx_copy_to_enclave(const void* ptr, uint64_t maxsize, const void* uptr, uint64_t usize);
 
 int sgx_report (sgx_arch_targetinfo_t * targetinfo,
                 void * reportdata, sgx_arch_report_t * report);

Some files were not shown because too many files changed in this diff