New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerabilities in SGX trusted enclave_entry procedure #28

Closed
jovanbulck opened this Issue Dec 20, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@jovanbulck
Contributor

jovanbulck commented Dec 20, 2016

I identified several security-critical vulnerabilities in Graphene's trusted runtime for SGX enclaves. It should be noted that the containing host application, including the unprotected Graphene runtime, is completely untrusted in the SGX attacker model.

Vulnerabilities

As a general rule, the trusted intra-enclave runtime should properly check all arguments/return values passed from the untrusted runtime. This is currently not the case in at least the following places:

  • enclave_entry.S: does not perform bounds checking on the provided index for the entry function lookup table. The untrusted runtime can overflow/underflow the ecall_table lookup by providing an out-of-bounds entry index in %rdi:
.Lhandle_ecall:
	lea ecall_table(%rip), %rbx
	mov (%rbx,%rdi,8), %rbx
        ...
	call *%rbx

An attacker controlling the unprotected application can redirect control flow to non-entry functions by abusing intra-enclave function pointers. She could even jump to arbitrary in-enclave code by over/underflowing the ecall_table lookup to unprotected memory (or enclave memory that holds user input).

  • enclave_entry.S: allows to "return" into an enclave thread that is not waiting for a previous ocall return. Untrusted runtime can simply provide the special return entry index in %rdi:
	cmp $RETURN_FROM_OCALL, %rdi
	je .Lreturn_from_ocall
        ...
.Lreturn_from_ocall
	mov %gs:SGX_LAST_STACK, %rsp
        popfq
        ...
        ret

An attacker can abuse this to initialize the in-enclave stack pointer of a newly started thread with the value of the last ocall. The memory content at these locations determine register values, and ultimately control flow.

  • enclave_ecalls.c contains an entry function enclave_ecall_thread_start that redirects control flow to a provided function address. The function pointer is not restricted in any way, allowing an attacker to jump to arbitrary code locations within the enclave. The attacker also has control over the first argument provided to the function.

Proof-of-Concept Exploit

I included a proof-of-concept exploit at https://github.com/jovanbulck/graphene.

To demonstrate the danger of allowing arbitrary non-entry code execution, I wrote an elementary application that decides access to an access_allowed function by comparing a user-provided number with a secret PIN code. After confirming the application has been loaded, the untrusted runtime abuses the ecall_table lookup in enclave_entry.S to call access_allowed, regardless of the provided PIN. This shows that an attacker can redirect control flow to arbitrary enclaved code, including the loaded application binary:

Hello world from enclaved application binary!
	--> ac.allowAccess at 0x601078 is 0x4007e1 (access_allowed_handler)
app: enter PIN..
> 1233
user entered 1233
app: checking acess..
===> app: access denied! <===
app: ocall: opening dummy_file

[urts] entering enclave with illegal ecall idx -24332361 (ecall_table_adrs=0xbfa52c0)
enclave_handle_ecall: asm stub passed function pointer:
[ocall_dump] 0x4007e1
===> app: access allowed! <===
destroying enclave...

I also included a proof-of-concept exploit that redirects control flow to an arbitrary non-entry function of the trusted runtime by abusing the argument of enclave_ecall_thread_start. It can easily be understood that this allows for code abuse attacks that could break the confidentiality/integrity of in-enclave data.

Security Patches

It should be clear from the above explanation and the exploit that Graphene's trusted runtime should restrict enclave entry to a few well-defined entry points. I did not write a patch since it is not yet entirely clear what the best solution would be:

  • enclave_entry.S: given the security-sensitive role of the entry assembly code, I suggest reducing it to the absolute minimum. After switching to the secure stack, a normal C function could be called to take care of the ecall/return from ocall. At this point, entry index bounds checking and illegal returns could be properly handled. This approach resembles the official SGX SDK, where an enter_enclave C function is called from the enclave_entry asm stub.

  • enclave_ecalls.c: the current implementation of thread_start is fundamentally flawed. Entry code execution should be limited to predefined entry points. It is unclear to me how this fits in Graphene's untrusted runtime/glibc threading model (?)

Regards,

<jo.vanbulck at cs.kuleuven dot be>

@chiache

This comment has been minimized.

Member

chiache commented Dec 20, 2016

Thanks for pointing out the vulnerabilities! The detailed report is very helpful--I seriously wish there is a way to further acknowledge your contribution. I believe this issue has ultimate urgency and should be addressed with the top priority.

chiache added a commit that referenced this issue Feb 11, 2017

Bugfixes:
- Fixing AES-CMAC algorithm
- Using SHA512 to hash file stubs (much faster than SHA256 and AES-CMAC)
- Hardening enclave interface (still work-in-progress); delt with issue #28
- Handling socket/pipe polling better
- Allowing setting the lowest heap address in enclaves ('sgx.heap_min' in manifest)
- Fixing the pipe between processes (for SGX)
- Fixing race condition in futex handling in LibOS
- Inheriting epoll handles in forked chilren
- Assigning signal code (now only hard-coding FPE_INTDIV, BUS_ADRERR, SEGV_ACCERR, SEGV_MAPERR)
- Fixing race condition in vma allocation (likely to fix bug() in bookkeep/shim_vma.c)
- Clearer debug message in LibOS
- Fixing calling convention in LibOS and glibc
- Fixing futex behavior (FUTEX_WAIT takes relative time, FUTEX_WAIT_BITSET takes absolute time)
- More system call implemented
- More application working (NGINX)
@chiache

This comment has been minimized.

Member

chiache commented Feb 11, 2017

The vulnerabilities are addressed in the latest commit. See the changes in enclave_entry.S. Again, I want to thank your contribution and effort to investigating these vulnerabilities.

@chiache chiache closed this Feb 11, 2017

@donporter

This comment has been minimized.

Member

donporter commented Feb 12, 2017

Jo: Thank you again for the report. In the event any similar vulnerabilities find their way into our code, please feel free to send more reports our way.

donporter added a commit that referenced this issue Aug 13, 2018

Bugfixes:
- Fixing AES-CMAC algorithm
- Using SHA512 to hash file stubs (much faster than SHA256 and AES-CMAC)
- Hardening enclave interface (still work-in-progress); delt with issue #28
- Handling socket/pipe polling better
- Allowing setting the lowest heap address in enclaves ('sgx.heap_min' in manifest)
- Fixing the pipe between processes (for SGX)
- Fixing race condition in futex handling in LibOS
- Inheriting epoll handles in forked chilren
- Assigning signal code (now only hard-coding FPE_INTDIV, BUS_ADRERR, SEGV_ACCERR, SEGV_MAPERR)
- Fixing race condition in vma allocation (likely to fix bug() in bookkeep/shim_vma.c)
- Clearer debug message in LibOS
- Fixing calling convention in LibOS and glibc
- Fixing futex behavior (FUTEX_WAIT takes relative time, FUTEX_WAIT_BITSET takes absolute time)
- More system call implemented
- More application working (NGINX)

donporter added a commit that referenced this issue Oct 24, 2018

Bugfixes:
- Fixing AES-CMAC algorithm
- Using SHA512 to hash file stubs (much faster than SHA256 and AES-CMAC)
- Hardening enclave interface (still work-in-progress); delt with issue #28
- Handling socket/pipe polling better
- Allowing setting the lowest heap address in enclaves ('sgx.heap_min' in manifest)
- Fixing the pipe between processes (for SGX)
- Fixing race condition in futex handling in LibOS
- Inheriting epoll handles in forked chilren
- Assigning signal code (now only hard-coding FPE_INTDIV, BUS_ADRERR, SEGV_ACCERR, SEGV_MAPERR)
- Fixing race condition in vma allocation (likely to fix bug() in bookkeep/shim_vma.c)
- Clearer debug message in LibOS
- Fixing calling convention in LibOS and glibc
- Fixing futex behavior (FUTEX_WAIT takes relative time, FUTEX_WAIT_BITSET takes absolute time)
- More system call implemented
- More application working (NGINX)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment