Skip to content
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

Security issue #19

Open
yudhui opened this issue Nov 23, 2021 · 2 comments
Open

Security issue #19

yudhui opened this issue Nov 23, 2021 · 2 comments

Comments

@yudhui
Copy link

yudhui commented Nov 23, 2021

Hi,
We found a buffer overflow and a infomation leak in Enclave/Enclave.cpp.
There is a global variable "Arg arg_enclave;" in enclave and it is initialized in ecall "enclave_init_values". However it value is copyed from "arg" which is untrusted.

image

We found a member variable in structure "Arg" called "int max_buf_size;". Then we search the code to find where "max_buf_size" is used. We found two patterns:
First is like this "memset(cipher, 0, arg_enclave.max_buf_size);". For example it is used in "enclave_get", while the parametre "cipher" refers to the buf in "enclave_process", and its size is a constant(4125). So, arg_enclave.max_buf_size can be larger than the buffer size.
Second pattern is "message_return(cipher, arg_enclave.max_buf_size, client_sock);". Function "message_return" is an ocall which writes cipher to client_sock.

image

In function "enclave_message_pass" cipher is introduced from data which is original from "HotCall* hotEcall". While "hotEcall" is transferd from untrusted part.

image

So, attacker can set cipher points to arbitrary address in encalve and set client_sock to sdtout that will leak information in encalve.

Thanks,
yudhui

@cocoppang
Copy link
Owner

Hi yudhui,

As I understand, you pointed out 3 issues.

  1. untrusted configurations passed to enclave
  • Definitely yes, to implement a fully trusted key-value store, it requires elaborate handling for the configurations. Since ShieldStore is a prototype of trusted key-value store, we implemented it as simply passing the untrusted arguments.
  1. buffer overflow due to the mismatch of buffer size
  • The mismatch of the buffer size can cause buffer overflow as you mentioned. I think our experimental code contains the constant size of buffer (BUF_SIZE), but there’s no use of BUF_SIZE in the main code.
  1. A vulnerable point for an untrusted buffer
  • Since the “cipher” is the part of untrusted memory address, an attacker can freely manipulate the contents or address of the buffer. As you described, if the attack arbitrary points the “cipher” to an enclave address, the information leakage can be occurred. One of the simple solutions is to allocate a new buffer for the response in order to decouple the buffers.

Thank you for the consideration what we didn’t catch.

Taehoon

@yudhui
Copy link
Author

yudhui commented Jan 20, 2022

Hi, Taehoon,

Thank you for your reply. I think the solutions you mentioned can fix the security issues. Do you have any patching schedule?

Thanks,
yudhui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants