OpenVPN fuzzers released + notes

First of all a very heartfelt thanks to all those who donated in the wake of my OpenVPN findings.

Private Internet Access donated $1000! Another donor whose I identity I know is Shawn C[ a.k.a “citypw”] of HardenedLinux. An unknown person or company donated $1000 as well. And about 15 others donated too. Thank you so much! Very inspiring and your generous tokens of appreciation mean a lot to me.

I’ve uploaded my fuzzers here.

Maybe you think that OpenVPN has now been completely X-rayed by fuzzers; this is not the case.  There are still signification portions of the code that are left to explore. It wouldn’t surprise me at all if more vulnerabilities emerge. This requires that more fuzzers are written for specific parts (for example all the code in ssl.c !). I have disabled some ASSERTs and other code because they crash the fuzzer, for example here, and it requires more research to determine whether these asserts are reachable remotely. The fuzzing framework + IO abstractions I have published will make the creation of more fuzzers a relatively easy effort, as long as you know what you are doing (set variables passed to a function to sane values, comment out code where applicable, etc).

If you find more vulnerabilities, report them to security@openvpn.net before you disclose them and enjoy your bragging rights and maybe a bounty from OSTIF.

There appears to be an impression that I’m in favor of abolishing manual code reviews. This is not true. In fact, I listed several types of vulnerabilities that cannot be found with fuzzers. All I’m saying is to be aware of the respective strengths and weakness of each strategy, and to use a sensible approach to find vulnerabilities. If you have a function that takes a null-terminated a string followed by 500 lines of spaghetti code that does string operations like it’s 1983, I’d rather write a fuzzer in 3 minutes that finds 5 overflows in 5 minutes, than frying my brain for an hour to discern the logic and manually concocting input that leads to corner cases. Of course, you can still do a manual examination once the fuzzer has run its course. But a fuzzer isn’t going to detect that you’re accidentally sending your private key to the peer due to program logic gone awry, and instead requires human intelligence determine this. So the upshot is; use both, and be sensible in striking a balance.

Interestingly, the wrong method to free a GENERAL_NAMES structure is not limited to OpenVPN. By using the fantastic Debian Codesearch I discovered that the issue was also present in FreeRADIUS and StrongSwan. Maybe that’s just the tip of the iceberg.

There’s some cool stuff I’m working on (not related to OpenVPN); check back here in a few days.

The OpenVPN post-audit bug bonanza

UPDATE: OpenVPN fuzzers now released.

Summary

I’ve discovered 4 important security vulnerabilities in OpenVPN. Interestingly, these were not found by the two recently completed audits of OpenVPN code. Below you’ll find mostly technical information about the vulnerabilities and about how  I found them, but also some commentary on why commissioning code audits isn’t always the best way to find vulnerabilities.

Here you can find the latest version of OpenVPN: https://openvpn.net/index.php/open-source/downloads.html

This was a labor of love. Nobody paid me to do this. If you appreciate this effort, please donate BTC to 1D5vYkiLwRptKP1LCnt4V1TPUgk7cxvVtg.

Introduction

After a hardening of the OpenVPN code (as commissioned by the Dutch intelligence service AIVD) and two recent audits 1 2, I thought it was now time for some real action ;).

Most of this issues were found through fuzzing. I hate admitting it, but my chops in the arcane art of reviewing code manually, acquired through grueling practice, are dwarfed by the fuzzer in one fell swoop; the mortal’s mind can only retain and comprehend so much information at a time, and for programs that perform long cycles of complex, deeply nested operations it is simply not feasible to expect a human to perform an encompassing and reliable verification.

End users and companies who want to invest in validating the security of an application written in an “unsafe” language like C, such as those who crowd-funded the OpenVPN audit, should not request a manual source code audit, but rather task the experts with the goal of ensuring intended operation and finding vulnerabilities, using that strategy that provides the optimal yield for a given funding window.

Upon first thought you’d assume both endeavors boil down to the same thing, but my fuzzing-based strategy is evidently more effective. What’s more, once a set of fuzzers has been written, these can be integrated into a continuous integration environment for permanent protection henceforth, whereas a code review only provides a “snapshot” security assessment of a particular software version.

Manual reviews may still be part of the effort, but only there where automation (fuzzing) is not adequate. Some examples:

  • verify cryptographic operations
  • other application-level logic, like path traversal (though a fuzzer may help if you’re clever)
  • determine the extent to which timing discrepancies divulge sensitive information
  • determine the extent to which size of (encrypted) transmitted data divulges sensitive information (see also). Beyond the sphere of cryptanalysis, I think this is an underappreciated way of looking at security.
  • applications that contain a lot of pointer comparisons (not a very good practice to begin with — OpenVPN is very clean in this regard, by the way) may require manual inspection to see if behavior relies on pointer values (example)
  • can memory leaks (which may be considered a vulnerability themselves) can lead to more severe vulnerabilities? (eg. will memory corruption take place if the system is drained of memory?)
  • can very large inputs (say megabytes, gigabytes, which would be very slow to fuzz) cause problems?
  • does the software rely on the behavior of certain library versions/flavors? (eg. a libc function that behaves a certain way with glibc may behave differently with the BSD libc — I’ve tried making a case around the use of ctime() in OpenVPN)

So doing a code audit to find memory vulnerabilities in a C program is a little like asking car wash employees to clean your car with a makeup brush. A very noble pursuit indeed, and if you manage to complete it, the overall results may be even better than automated water blasting, but unless you have infinite funds and time, resources are better spent on cleaning the exterior with a machine, vacuuming the interior followed by an evaluation of the overall cleanliness, and acting where necessary.

Vulnerabilities

Remote server crashes/double-free/memory leaks in certificate processing

Reported to the OpenVPN security list on May 13.

CVE-2017-7521

There are several issues in the extract_x509_extension() function in ssl_verify_openssl.c. This function is called if the user has used the ‘x509-username-field’ directive in their configuration.

GENERAL_NAMES *extensions;
int nid = OBJ_txt2nid(fieldname);

extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL);

The first issue. The ‘fieldname’ variable is the value specified in the configuration file after the ‘x509-username-directive’. Different NID’s require different storage structures. That is to say, using a GENERAL_NAMES structure for every NID will result in spectacular crashes for some NIDs.


ASN1_STRING_to_UTF8((unsigned char **)&buf, name->d.ia5);
if (strlen(buf) != name->d.ia5->length)
{
    msg(D_TLS_ERRORS, "ASN1 ERROR: string contained terminating zero");
    OPENSSL_free(buf);
}
else
{
    strncpynt(out, buf, size);
    OPENSSL_free(buf);
    retval = true;
}

The second issue. The return value of ASN1_STRING_to_UTF8 is not checked. It may return failure, in which case buf retains its value. This code is executed in a loop (for every GENERAL_NAME encoded in the certificate). So let’s consider this scenario:

First loop: ASN1_STRING_to_UTF8 succeeds, and buf is processed and freed in any of the following branches.
Second loop: ASN1_STRING_to_UTF8 fails, and buf is processed (use-after-free) and freed (double-free) in any of the following branches.

In spite of extensive fuzzing I could not trigger a single ASN1_STRING_to_UTF8 failure using OpenSSL 1.0.2l. It may or may not be possible with other versions of OpenSSL, LibreSSL, BoringSSL. This would NOT indicate a bug in those libraries — as an API, they are allowed to fail for any reason. The actual error is OpenVPN not checking the return value.

But what makes this interesting is that at the end of this function, the following attempt is made to free the ‘extensions’ variable.

sk_GENERAL_NAME_free(extensions);

This is wrong. The correct way to do this is to call GENERAL_NAMES_free. This is because sk_GENERAL_NAME_free frees only the containing structure, whereas GENERAL_NAMES_free frees the structure AND its items.

Hence, there is a remote memory leak here.

If you look in the OpenSSL source code, one way through which ASN1_STRING_to_UTF8 can fail is if it cannot allocate sufficient memory. So the fact that an attacker can trigger a double-free IF the server has insufficient memory, combined with the fact that the attacker can arbitrarily drain the server of memory, makes it plausible that a remote double-free can be achieved. But if a double-free is inadequate to achieve remote code execution, there are probably other functions, whose behavior is wildly different under memory duress, that you can exploit.

Furthermore, there are two more instances of ASN1_STRING_to_UTF8 in this file:

(in the function extract_x509_field_ssl)

tmp = ASN1_STRING_to_UTF8(&buf, asn1);
if (tmp <= 0) {    return FAILURE; } 

(in the function x509_setenv_track)

 if (ASN1_STRING_to_UTF8(&buf, val) > 0)
{
    do_setenv_x509(es, xt->name, (char *)buf, depth);
    OPENSSL_free(buf);
}

(in the function x509_setenv)

if (ASN1_STRING_to_UTF8(&buf, val) <= 0)
{
    continue;
}

Here, the code assumes that a return value that is negative or zero indicates failure, and ‘buf’ is not initialized, and needs not to be freed. But in fact, this is ONLY the case if ASN1_STRING_to_UTF8 returns a negative value. A return value 0 simply means a string of length 0, but memory is nonetheless allocated, so there are memory leaks here as well.

Remote (including MITM) client crash, data leak

Reported to the OpenVPN security list on May 19.

CVE-2017-7520

This only affects clients who use OpenVPN to connect to an NTLM version 2 proxy.

ntlm_phase_3() in ntlm.c:

if (( *((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)          /* Check for Target Information block */
{
    tib_len = buf2[0x28];            /* Get Target Information block size */
    if (tib_len > 96)
    {
        tib_len = 96;
    }
    {
        char *tib_ptr = buf2 + buf2[0x2c];           /* Get Target Information block pointer */
        memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len);           /* Copy Target Information block into the blob */
    }
}

‘buf2’ is an array of type char (signed), which contains data sent by the peer (the proxy).
‘tib_len’ is of type int.

First issue: remote crash. If buf[0x28] contains a value of 0x80 or higher, ‘tib_len’ will be negative; both variables are signed, after all. This will cause memcpy to crash.

Second issue: data leak. buf[0x2c] is used as an index to the buf2 array. Because ‘buf2[0x2c]’ is a signed value, if it is >= 0x80, it will cause tib_ptr to point BEFORE ‘buf2’.
Memory at this location is then copied to ntlmv2_blob, which is then sent to the peer.
This constitutes a data leak.

Because the user’s password is also stored on the stack (the variable ‘pwbuf’ in this function), this or other sensitive information to the peer in cleartext.

These issues can be triggered by an actor in an active man-in-the-middle role.

Remote (including MITM) client stack buffer corruption

Reported to the OpenVPN security list on June 6.
No CVE
Patch: https://github.com/OpenVPN/openvpn/commit/69162924de3600bfe8ae9708a1d6e3f4515ef995

This is exceedingly unlikely to occur.

The my_strupr function in ntlm.c is constructed as follows:

unsigned char *
my_strupr(unsigned char *str)
{
    /* converts string to uppercase in place */
    unsigned char *tmp = str;

    do
    {
        *str = toupper(*str);
    } while (*(++str));
    return tmp;
}

From this code it is obvious that if a string of length 0 is passed, OOB read(s) and possibly write(s) will occur.

In the case of a string of length 0, the null terminator is toupper()’ed, pointer is incremented, byte AFTER null terminator is evaluated, and if not null toupper()’ed, until a second NULL byte is seen.

The function is invoked once:

my_strupr((unsigned char *)strcpy(userdomain, username));

Exploitation can only be achieved if:

  • NTLM version 2 is used.
  • The user specified a username ending with a backslash.
  • The (uninitialized) ‘username’ array constists entirely of non-null values.
  • The stack layout is such that the ‘username’ array is followed by a pointer, or something else that, if toupper()’ed, could cause arbitrary code execution.

This issue can be triggered by an actor in an active man-in-the-middle role.

Remote server crash (forced assertion failure)

Reported to the OpenVPN security list on May 20.
CVE-2017-7508

The OpenVPN server can be crashed by sending crafted data.

mss_fixup_ipv6() in mss.c:

if (buf_advance( &newbuf, 40 ) )
{
    struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *) BPTR(&newbuf);
    if (tc->flags & OPENVPN_TCPH_SYN_MASK)
    {
        mss_fixup_dowork(&newbuf, (uint16_t) maxmss-20);
    }
}

in mss_fixup_dowork():

ASSERT(BLEN(buf) >= (int) sizeof(struct openvpn_tcphdr));

It is possible to construct a packet to the server such that this assertion will fail, and the server will stop.

Crash mbed TLS/PolarSSL-based server

Reported to the OpenVPN security list on May 22.

CVE-2017-7522

This requires that the –x509-track configuration option has been set. It affects OpenVPN 2.4 (not 2.3) compiled with mbed TLS/PolarSSL as the cryptography backend. The crafted certificate must have been signed by the CA.

When parsing the client certificate, asn1_buf_to_c_string() may be called (via x509_setenv_track -> do_setenv_name).
It iterates over an ASN1 string as follows:

for (i = 0; i < orig->len; ++i)
{
    if (orig->p[i] == '\0')
    {
        return "ERROR: embedded null value";
    }
}

If a null byte is found within this string (ASN1 allows this), the static string “ERROR: embedded null value” is returned. If no null byte is found, a heap-allocated string is returned.

The static string becomes problematic if a while later string_mod() is called. This attempts to modify the string. This will typically cause a crash, because the static string is stored in a read-only memory region.

Stack buffer overflow if long –tls-cipher is given

Reported to the OpenVPN security list on May 12
No CVE
Patch: https://github.com/OpenVPN/openvpn/commit/e6bf7e033d063535a4414a4cf49c8f367ecdbb4f

An excessively long –tls-cipher option can cause stack buffer corruption. This can only affect the user if they load untrusted options. Not considered an actual vulnerability because untrusted options may execute arbitrary code via other option directives by design (see commit message).

As a general rule, don’t load untrusted configuration files.

(v)s(n)printf hardening

Reported to the OpenVPN security list on May 23

This is not a vulnerability. It is a proposed hardening technique. My motivation can be read here: https://community.openvpn.net/openvpn/ticket/894
The gist is that vsnprintf and related functions (upon which OpenVPN heavily relies) can, in theory, fail. The reasons for this are entirely inherent to the libc’s internal logic, and behavior may differ from one libc to the other. It must be noted that it is exceedingly unlikely that these functions fail in practice. However, should this happen, this could create dangerous data leaks of sensitive data. My proposed patch remedies this and ensures no data is ever leaked.

Other bugs

Some other minor bugs, that don’t impact security, have been found: https://github.com/OpenVPN/openvpn/commits/master

How I fuzzed OpenVPN

Fuzzing OpenVPN has been an extensive effort. You can’t just chain the fuzzer to arbitary internal functions for various reasons:

  • OpenVPN executes external programs like ipconfig and route to modify the system’s networking state. This is not acceptable within a fuzzing environment.
  • Direct resource access (files, networking) occurs throughout the code. You certainly don’t want the fuzzer to end up writing random files and sending data to random IP’s.
  • There are many ASSERT() statements throughout the code. These will cause a direct abort if the enclosed condition is false. This makes fuzzing impossible; you want the fuzzer to run for hours, not abort after 2 seconds.

To work around the first problem, I modified the source code such that in fuzzing mode, everything leading up to the actual execve() is executed (processing of arguments to the external program), but the actual execve() call is commented out. It will return success or failure based on a bit in the fuzzer input data.

To prevent access to resources, I implemented abstractions for libc functions. For example, recv() is now platform_recv(), and within platform_recv() I either call recv() directly (in non-fuzzing mode), or grab data from the fuzzer input data (in fuzzing mode). Similarly, through abstractions such as platform_read(), the application can open, read and write to files at will. The data that it expects is transparently pulled from the fuzzing input.

To deal with the assertions, there is no other way than to comment them out (#ifndef FUZZING .. ASSERT(condition) .. #else if (condition) return; #endif), but only in certain cases.

I leave them in place in situations where the assertion condition depends directly on untrusted data. As an example, say the application recv()’s data from the peer, and then does ASSERT(recvd_data[3] == 0x20). It is important to leave this ASSERT in; it implies that the client can force an abort() on the server (or vice-versa); this can be considered a security issue.

But there are also ASSERTs that rely on variables within an internal data structure. I typically fill these data structures with fuzzer input. Rather than manually ensuring that these variables are valid and coherent with regard to the application’s logic, I simply change the ASSERTs that rely on this validity into ‘return’ where possible (and free objects, where applicable).

I’ve used libFuzzer combined with AddressSanitizer (ASAN), UndefinedBehaviorSanitizer (UBSAN) and MemorySanitizer (MSAN). ASAN cannot be combined with MSAN, and moreover MSAN does not work with libFuzzer (due to the apparent use of uninitialized memory within libFuzzer itself). So the way to go is to generate a corpus with the fuzzer, and then execute each of the resulting inputs with a MSAN-enabled standalone version.

There are various discrete components in OpenVPN that together constitute the application. There is an extensive suite of functions to deal with data buffers (buffer.c, buffer.h), an extensive option parser (options.c — parses the configuration file, command line arguments as well as commands pushed by server to client), a base64 encoder/decoder, etc.
Thanks to this relative modularity in OpenVPN it has been possible to use and abuse these components as if they were an API with relatively little effort.
My approach for testing all of these API-like components is as follows:

(assume 3 functions to be tested)

  • Get a number from the fuzzer input data in the range 0 – 2.
  • Call either of the three functions based in the number
  • Provide each function with parameters derived from the input data where dynamic parameters are required
  • Repeat the above process a number of times (for example (for i = 0; i < 5; i++) { … })

This will cause an ever-permutating sequence of invocations. Essentially the coverage surface becomes (near-) absolute, that is to say, (almost) every conceivable way to use the API is a contender to be tested via this algorithm.

This approach is especially useful to test the functions that operate on the same structure. If there exists any sequenced set of functions that would cause memory violations, this setup is bound to find it. Of course, the actual use of any group of functions within the application is only a small subset of all permutations and parameters that the fuzzer sets out to test, and any mishaps the fuzzer finds for very particular circumstances may not actually occur within the code. But it is nonetheless good to know, because:

  • If you know that a certain sequence of calls and their parameters will lead to memory corruption, you can now perform a manual code analysis to see if this situation occurs.
  • Corner-case API bugs that are not invoked now, may become manifest in the future once code (with calls to the API) is added that does trigger these bugs.

In MSAN-enabled builds I serialize the output structure (if there is one) to /dev/null. For example, the options parser stores all its data in a struct options variable. MSAN does not immediately report the use of uninitialized data; it only does so if it is used in conditions that lead to branching (if (x) …) or when the data is used for I/O.

Hence, by serializing this data to /dev/null (normally a no-op), I force MSAN to detect uninitialized data. In C, there is no automatic way to serialize nested data structures (struct A contains a pointer to struct B etc), so for some structures I had to manually make a serialization stack of functions.

Limited fuzzing on a 32 bit platform has also been performed. This did not find any issues that do not occur on 64 bit.

rpcbomb: remote rpcbind denial-of-service + patches

UPDATE: A CVE number has been assigned, it’s: CVE-2017-8779.

This vulnerability allows an attacker to allocate any amount of bytes (up to 4 gigabytes per attack) on a remote rpcbind host, and the memory is never freed unless the process crashes or the administrator halts or restarts the rpcbind service.

Attacking a system is trivial; a single attack consists of sending a specially crafted payload of around 60 bytes through a UDP socket.

This can slow down the system’s operations significantly or prevent other services (such as a web server) from spawning processes entirely.

This vulnerability may also be conducive in exploiting unrelated software running on the target system; there is a class of bugs that become exploitable (using uninitialized buffers, double frees, out-of-bounds writes, what have you) when there is insufficient memory; a call to malloc() fails and the software starts behaving in a way that was not foreseen. Resource shortage is an otherwise rare situation, so a lot of software is not equipped to safely deal with this situation.

It might not be easy to turn this constellation of factors into a practical exploit, but this class of bugs is a real thing, and it is the reason why a project like OpenSSL is now simulating allocation failures in their fuzzing framework, while applications like Tor will immediately terminate upon an allocation failure.

Suffice it to say that this is a potentially severe vulnerability.

I’ve developed a fix for this bug. This involves patching both rpcbind and libtirpc. I’ve been in touch with the maintainer of both packages but communication is slow so I’ve decided to go ahead and publish my findings. I see no reason to withhold them any longer because I’m including the remedy which distributions can consume and push to their users. You are encouraged to verify the correctness of the patches yourself before merging.

Exploit and patches can be found here: https://github.com/guidovranken/rpcbomb

shodan_rpcbind

Shodan reports 1.8 million hosts serving on port 111 (rpcbind). Many of those appear to be Amazon AWS instances and other mass hosting services where the owner is presumably using their default Linux distribution configuration that leaves rpcbind open to the internet. I assume there’s also a large amount of rpcbind services running behind internet firewalls (so they go undetected by Shodan) but accessible via their local networks or within a VPN.

Patch today or better yet, unless you really need it, remove the rpcbind service entirely from your system. If you really need it, it might be wise to apply some access limitation to port 111 in your firewall. I’ve scrutinized rpcbind and libtirpc to the best of my ability, but it is possible that this old software still hides vulnerabilities of equal or even greater severity.

A question to my readers

With regards to the class of vulnerabilities mentioned earlier, where resource shortage can lead to, I’d like to ask my dear readers a question: should memory-corrupting or memory-divulging bugs, which only exist under memory shortage that is non-trivial (assume a failure to allocate a couple of hundred kilobytes or more is the lower bound required to trigger the bug), be taken seriously and treated as proper vulnerabilities? The reason I’m asking this is because instances of this class of bugs are usually met with some ambivalence; let’s make a fix because who knows, and bury it in the endless stream of other nondescript commits, and let’s not issue an advisory because the prerequisites are outlandish. Failing to allocate 500 kilobytes on systems that are not a Commodore 64 is unusual,  but obviously and inevitably there is a limit to the free memory that can be consumed. The amount of memory that is free at any given moment is largely outside the software’s purview; ultimately, memory is a shared and limited resource, and every concurrent process can affect the others’ stability.

Actual exploit development is not my forte nor my objective, so I’ve put up this survey to collect the opinions of those who are perhaps better versed in this area.

Take into account the fact that some systems like embedded devices operate in a state (default memory consumption, upper bound of memory consumption, processes running at any given moment) that is more or less known to the attacker, which may benefit the precision and reliability of an exploit.

I will post the results of this survey!

A script to find call recursions in binaries

https://github.com/guidovranken/binloop

This is a script I’ve slapped together to find potential call recursions in binaries. A call recursion occurs when functions comprise a loop; for instance, function a() calls function b(), and function b() calls function a(). If the recursion depth isn’t properly curtailed, this can lead to a stack overflow, which crashes the program and is essentially a denial-of-service. Especially for server software that can enter a deep recursion upon the instigation of an untrusted client, this is problematic, as it essentially constitutes a remote DoS. Thanks to this program I once found a remote DoS vulnerability in Apache: CVE-2015-0228.
An other way to do this is to use a static C/C++ analyzer to parse the entire source code and find recursion loops on the resulting invocation tree, but this approach saves you the parsing overhead and operates on the end product (the binary) instead.
This script implements my own algorithm to find loops in a graph. I believe scipy offers such an algorithm out-of-the-box, so you may use that instead if it is faster.
The script does not find loops that involve function pointers; this is far more difficult to detect as it requires a semantic understanding of the code.
It is configured to output 25 loops at most, to prevent that excessive output is generated (this happens for some software, try Python or Tor). Change it to suit your needs.
You will probably need to alter it a bit if you’re going to use it for non-x64 binaries.

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.