bl4ck5un / mbedtls-SGX

mbedtls-SGX: a SGX-friendly TLS stack (ported from mbedtls)
Apache License 2.0
91 stars 32 forks source link

[Security] Information leak mbedtls_ssl_flush_output() #16

Closed mustakimur closed 5 years ago

mustakimur commented 5 years ago

There is a massive information leak case in mbedtls_ssl_flush_output() using the ocall_mbedtls_net_send(). The vulnerable code is here:

while( ssl->out_left > 0 )
    {
        MBEDTLS_SSL_DEBUG_MSG( 2, ( "message length: %d, out_left: %d",
                       mbedtls_ssl_hdr_len( ssl ) + ssl->out_msglen, ssl->out_left ) );

        buf = ssl->out_hdr + mbedtls_ssl_hdr_len( ssl ) +
              ssl->out_msglen - ssl->out_left;
        ret = ssl->f_send( ssl->p_bio, buf, ssl->out_left );

        MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", ret );

        if( ret <= 0 )
            return( ret );

        ssl->out_left -= ret;
    }

Here, ssl->out_left is size_t (unsigned) and the ret is ocall return (hence untrusted). The ssl->out_left -=ret; line can cause ssl->out_left to a large positive integer (hence infinite loop). Moreover, the buf is pointed to memory using the untrusted ssl->out_left can let it pointing to any memory location and using the next ocall, dump the entire enclave memory. Note, ret is only check for negative value filter. A value greater than ssl->out_left can cause this issue.

bl4ck5un commented 5 years ago

That's a good point. There needs to be a check that and buf and buf + ssl->out_left is within the range of the outgoing buffer. Would you like to submit a fix? Or I can work on it later too.

mustakimur commented 5 years ago

I saw there is a fix for this in the main project. I will clone that here soon.