search

lsds / TaLoS

Efficient TLS termination inside Intel SGX enclaves for existing applications
Apache License 2.0
106 stars 20 forks source link

security issues due to prevalent use of [user_check] #18

Open david-oswald opened 5 years ago

david-oswald commented 5 years ago

This issue is related to #13:

In the TaLoS codebase, [user_check] is widely used (there are 369 instances of it in https://github.com/lsds/TaLoS/blob/master/src/talos/enclaveshim/enclave.edl). However, in most cases these pointers are never checked for inside/outside enclave.

The fix for #13 addressed one instance, but there are many more as far as I can see, for example:

https://github.com/lsds/TaLoS/blob/master/src/talos/patch/ssl_lib.c.patch#L694

This might also affect a lot of uses of SSL *s, as this pointer is also [user_check], and usually read from and written to (via https://github.com/lsds/TaLoS/blob/master/src/talos/patch/ssl_lib.c.patch#L725) without checking in most cases.

plaublin commented 5 years ago

Hello

Thank you for raising this issue. Indeed some additional checks could be implemented to check the provenance of the pointers passed with [user_check].

f0rki commented 5 years ago

Hi everyone, Hi @plaublin,

I am an academic security researcher at University of Duisburg-Essen, working with (@ldavi and @Tcc100). During our research on SGX we also discovered the security issues mentioned in this issue. However, we took it one step further and constructed a proof-of-concept exploit that achieves arbitrary code execution inside of the TaLoS SGX enclave, effectively bypassing any protections offered by SGX.

Indeed some additional checks could be implemented to check the provenance of the pointers passed with [user_check].

It is not easy to add sufficient checks to pointers passed with [user_check]. For example, a check for inside/outside enclave memory can often be bypassed by an attacker.

In our experience the best way to fix this is to avoid passing pointers to enclave memory at all and otherwise rely on the SGX SDK provided [in, out] attributes. Our suggestion is to replace pointers to enclave memory with simple integer identifiers, which are then translated into pointers using e.g., a hashmap or array lookup at the start of the ECALLs.

If you prefer I can contact you privately (e.g., via E-Mail) for details on our exploit and our recommendations on how to fix this issue.

plaublin commented 5 years ago

Hello

This is interesting. Would you have any reference to your proof of concept?

Replacing pointers with integer identifiers is what the shadow structures mechanism is doing: https://www.doc.ic.ac.uk/research/technicalreports/2017/DTRS17-5.pdf . We have used it for the SSL structure primarily, but it can be generalized to all of them.

Unfortunately I lack resources to quickly fix the issue... If you would happen to have pull requests I would be immensely grateful.

f0rki commented 5 years ago

Hi @plaublin, I'd rather discuss this privately. Can you contact me via email at michael.rodler (at) uni-due.de?

f0rki commented 4 years ago

For sake of completeness and since this probably won't get fixed any time soon, we published our PoC exploits (three variants) against this project: uni-due-syssec/teerex-exploits. Maybe this helps someone out there to create a pull request with fixes.