Security issue: missing return value check when invoking `ocall_mbedtls_net_accept`
Eadom opened this issue · 1 comments
I found a implementation issue in ocall_mbedtls_net_accept
that will casue a vulnerability.
This is the definition in EDL.
int ocall_mbedtls_net_accept( [in] mbedtls_net_context *bind_ctx, [out] mbedtls_net_context *client_ctx, [out, size=buf_size] void *client_ip, size_t buf_size, [out] size_t *ip_len );
As we known, all the value OCALL returns is untrusted. mbedtls_net_accept_ocall
is a wrapper function of OCALL function ocall_mbedtls_net_accept
. mbedtls_net_accept_ocall
should have checked whether the value returned from ocall_mbedtls_net_accept
is valid, E.g., the ip_len
should always less than or equal to buf_size.
Missing the check means the callee takes the responsibility to check returned value. Unfortunately, in example/enclave/s_server.c
, the sample code doesn't check the value returned from mbedtls_net_accept_ocall
, which will cause a stack memory leak.
int ssl_server()
{
...
unsigned char client_ip[16] = { 0 };
...
if( ( ret = mbedtls_net_accept_ocall( &listen_fd, &client_fd,
client_ip, sizeof( client_ip ), &cliip_len ) ) != 0 )
...
#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY)
if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
{
if( ( ret = mbedtls_ssl_set_client_transport_id( &ssl,
client_ip, cliip_len ) ) != 0 )
...
}
The code above invokes mbedtls_net_accept_ocall
to get client_ip
and cliip_len
at first, then it invokes mbedtls_ssl_set_client_transport_id
to store client_ip
with length of cliip_len
. The cliip_len
is returned from OCALL and there is no check on it. If the attacker returns cliip_len
that is larger than sizeof(client_ip)
, mbedtls_ssl_set_client_transport_id
will store larger size of contents than client_ip
should be. That's a stack memory leak.
To fix this issue, we can implement a wrapper function in enclave. It invokes mbedtls_net_accept_ocall
and check returned ip_len
.
Thanks. Good catch. Would you like to fix it and submit a PR?