bl4ck5un / Town-Crier

Town Crier: an Authenticated Data Feeds for Smart Contracts
https://town-crier.netlify.app/
Other
133 stars 25 forks source link

[security]information leakage in enclave code #69

Open jmp0x7c00 opened 2 years ago

jmp0x7c00 commented 2 years ago

hi,sir,

Town-Crier is an awesome project, I love it.but I think there maybe a security issue here , in file Enclave/SSLClient.c :

len = sizeof( buf ) - 1;
        memset( buf, 0, sizeof( buf ) );

        do ret = mbedtls_ssl_read( &ssl, buf, len );
        while( ret == MBEDTLS_ERR_SSL_WANT_READ ||
               ret == MBEDTLS_ERR_SSL_WANT_WRITE );

        if( ret <= 0 )
        {
            switch( ret )
            {
                case MBEDTLS_ERR_SSL_TIMEOUT:
                    mbedtls_printf( " timeout\n" );
                    if( retry_left-- > 0 )
                        goto send_request;
                    goto exit;

                case MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY:
                    mbedtls_printf( " connection was closed gracefully\n" );
                    ret = 0;
                    goto close_notify;

                default:
                    mbedtls_printf( " mbedtls_ssl_read returned -0x%x\n", -ret );
                    goto exit;
            }
        }

        len = ret;
        buf[len] = '\0';
// ===========================> leak here
        mbedtls_printf( " %d bytes read\n\n%s", len, (char *) buf );
        ret = 0;

Intel SGX can protect our sensitve data in enclave, however, we have to write our code carefully in case of leaking them by invoking ocall. I think we have to comment these lines before we use Town-Crier in release mode.

I think one solution is that we can define a DEBUG macro to decide wether to print them.

bl4ck5un commented 2 years ago

Hi @jmp0x7c00 , thanks for the note! The code is meant to be a research prototype for evaluation purposes so debugging statements like this is probably all over the place. Don't use this code in production!

That said, we do have logging level support in https://github.com/bl4ck5un/Town-Crier/blob/master/src/Enclave/log.h so you can modify logging to suite your needs.

mbedtls logging can be configured by

https://github.com/bl4ck5un/Town-Crier/blob/33471ff56cb75c9672a51c9d9c20352c96cc3444/src/Enclave/tls_client.cpp#L214

jmp0x7c00 commented 2 years ago

ok, I see. Thanks for your quick replay.!:) If we use Town-Crier in product environment, we have to modify its code a little.
here is an another leak-causing code, the decrypted message will be hexdumped outside enclave.

const string decrypt_query(const uint8_t* data, size_t data_len) {
  HybridEncryption dec_ctx;
  ECPointBuffer tc_pubkey;
  dec_ctx.queryPubkey(tc_pubkey);

  string cipher_b64(string(data, data + data_len).c_str());
  hexdump("encrypted query: ", data, data_len);

  try {
    HybridCiphertext cipher = dec_ctx.decode(cipher_b64);
    vector<uint8_t> cleartext;
    dec_ctx.hybridDecrypt(cipher, cleartext);

   // !!!===================> leak
    hexdump("decrypted message", &cleartext[0], cleartext.size());

    // decrypted message is the base64 encoded data
    string encoded_message(cleartext.begin(), cleartext.end());
    return encoded_message;
  }