
OpenSSL CVE-2016-0799: heap corruption via BIO_printf - 0x0
https://guidovranken.wordpress.com/2016/02/27/openssl-cve-2016-0799-heap-corruption-via-bio_printf/
======
narrator
This is one of those "let's ignore malloc failing" bugs. This is pretty common
in C code and is, IMHO, a common "worse is better"[1] anti-pattern, which is
predicated on the "worse is better" principle that correctness can be
sacrified if it makes things too complicated. Many programmers, unfortunately,
don't give a crap about things that far off the happy path, and when the
language implements checked exceptions or Option types they get upset that it
forces them to think about those distracting corner cases. Such is the state
of software engineering.

1\.
[https://en.m.wikipedia.org/wiki/Worse_is_better](https://en.m.wikipedia.org/wiki/Worse_is_better)

~~~
mattst88
The project I work on, Mesa3D, routinely ignores malloc failures for a couple
of reasons: no one knows how to reasonably test the additional code, and often
no one has any idea what sort of graceful failure is even possible when malloc
fails.

We've had some cargo culted patches to call _mesa_error_no_memory() when
malloc fails, but it was recently noted that this can't possibly work because
it internally calls fprintf, and there's no guarantee that fprintf doesn't
call malloc! In fact, glibc's implementation does.

Suggestions for best practices welcome.

~~~
CJefferson
If you don't have anything better to do, just abort().

One (naughty) option is to malloc a decent sized buffer at startup (50MB say),
and then when malloc fails free it, and immediately display a warning box.

One most modern 64-bit systems, malloc will never fail anyway, you'll get
killed due to memory overcommit.

~~~
caf
That's not so hotso for a library though, which Mesa3D is.

~~~
ori_b
The alternative, if you have no good way of testing the codepaths, or if
you're running on a system with overcommitted VM, is basically segfaulting.
And you're running on a system with overcommitted VM.

An abort with a reason is a _far_ better choice.

~~~
caf
Overcommit is not actually relevant. An overcommit OOM-kill is a manifestation
of the administrator killing the process for being a memory hog (it's just
that the administrator has delegated this decision to some less-than-scrutable
kernel code).

Library code should in general punt out-of-memory conditions back to the
caller.

~~~
ori_b
> _Overcommit is not actually relevant._

It is if you ever want to exercise the code paths. Overcommit means you never
get an out of memory error. Your process just dies. Kind of like it would if
you just aborted.

~~~
caf
When you're running tests in development you can easily set up a test
environment with and without overcommit enabled (and in fact there are other
ways to limit allocated memory on a per-process basis that work even with
overcommit, such as with rlimits).

...but that doesn't even really matter, because in order to fully exercise
those code paths you need precise control over exactly which call to malloc()
returns NULL, which in practice means you need to interpose malloc() with a
debugging framework to force a NULL return.

------
0x0
Three CVE's listed as currently vulnerable here: [https://security-
tracker.debian.org/tracker/source-package/o...](https://security-
tracker.debian.org/tracker/source-package/openssl)

With links to git commits:

CVE-2016-0705:
[https://git.openssl.org/?p=openssl.git;a=commit;h=ab4a81f69e...](https://git.openssl.org/?p=openssl.git;a=commit;h=ab4a81f69ec88d06c9d8de15326b9296d7f498ed)

CVE-2016-0798:
[https://git.openssl.org/?p=openssl.git;a=commit;h=59a908f1e8...](https://git.openssl.org/?p=openssl.git;a=commit;h=59a908f1e8380412a81392c468b83bf6071beb2a)

CVE-2016-0799:
[https://git.openssl.org/?p=openssl.git;a=commit;h=a801bf2638...](https://git.openssl.org/?p=openssl.git;a=commit;h=a801bf263849a2ef773e5bc0c86438cbba720835)

------
ComputerGuru
There was "fair notice" given last week (and front page'd here on HN) about an
upcoming release patching "several" vulnerabilities:
[https://mta.openssl.org/pipermail/openssl-
announce/2016-Febr...](https://mta.openssl.org/pipermail/openssl-
announce/2016-February/000063.html)

However, they're scheduled for release on Monday the 1st of March - so we
might be seeing a few more bugs in the next couple of days.

EDIT: I've spent all morning teasing a friend whose birthday is today about
"what if it were tomorrow" (I'm sure they get it every year from every single
person they meet), only to then come here and forget all about it in my tally.
Go figure. Yes, the 1st of March is Tuesday. Sorry!

~~~
Someone
Don't look for it on Monday; The 1st of March is Tuesday; 2016 is a leap year.

------
whoopdedo
That it has its own printf implementation, along with many other not-exactly-
crypto-related utilities, is a feature creep that I always found distasteful
about OpenSSL.

------
vszakats
"These are not the patches you are looking for":
[https://groups.google.com/forum/m/#!topic/mailing.openssl.de...](https://groups.google.com/forum/m/#!topic/mailing.openssl.dev/4m8
--XksQD4)

Clarification from OpenSSL maintainer.

------
viraptor
Nice code formatter. If in doubt, escape it thrice:

    
    
        _dopr(&amp;amp;amp;hugebufp, ....

~~~
iokanuon
That's actually four times - browser unescapes the first one.

    
    
        _dopr(&amp;amp;amp;amp;hugebufp, ...

------
alexandercrohde
Am I the only one who doesn't understand why it's still considered "okay" to
write C for security-dependent code these days? Processors have gotten order
of magnitudes faster, speed is no longer the constraining resource, and
certainly not one I would trade my security for...

~~~
serge2k
Oh my god what a revelation. I've never seen anyone bring this up before.

Outside of every single thread about C. Everyone at this point knows the
shitty side of C.

------
vmorgulis
It's a bit crazy to see again issues related to string manipulation. Why this
kind of stuff is not centralized yet?

~~~
stonogo
In the name of portability, they've opted to have low-quality NIHed code
instead.

------
wiredfool
This appears to be jumping the gun just a bit. They normally aim for during
the work week in the US?

------
viraptor
In case someone's interested in history, related change (return on malloc
fail) was done in March 2015 in a general alloc-fail cleanup across the
codebase.
[https://github.com/openssl/openssl/commit/918bb8652969fd53f0...](https://github.com/openssl/openssl/commit/918bb8652969fd53f0c390c1cd909265ed502c7e)

------
userbinator
From reading the description, I get the feeling that this is nowhere near as
severe as e.g. Heartbleed, right?

IMHO dynamic allocation is something that is often overused, and the fact that
it could fail leads to another error path that might be exploitable, like this
one. I don't think it's necessary for a printf-like function to _ever_ use
dynamic allocation. (If there are cases where it's necessary, I would love to
know more... and perhaps suggest how it could be avoided.)

------
latenightcoding
I arrived to the comments section before the C vs Rust flame war

~~~
topspin
Another day, another memory violation... the endless supply of ammo dispenses
another box of shells for Rust. I don't know if Rust is the future, but I am
certain the dangling pointers, use after frees, buffer overflows, double
frees, smashed stacks, ignored failures, uninitialized data and all the other
horrors of contemporary C/C++ will one day be understood as intolerable.

~~~
JoeAltmaier
These things can be reduced to nuisance level by discipline. The cost to
obviating them by language design is not zero (code size; execution speed;
random latencies) so its arguable, different languages, different purposes.

~~~
aschampion
> These things can be reduced to nuisance level by discipline.

Obviously not. We've been doing this for decades and are still making the same
mistakes with costly effects. Humans are as much a part of technological
systems as the language or platform; continuing to ignore that even educated,
disciplined, skilled developers are prone to certain classes of errors --
errors that could be obviated by other components of the system -- is hubris.
It is much more feasible to create tools that exclude certain classes of error
than to create less fallible developers.

> (code size; execution speed; random latencies)

Rust is not GC'ed, has no inherent reason to be slower than C (as the
toolchain matures), and I doubt is more verbose than C code that provides the
same safety.

~~~
JoeAltmaier
Speak for yourself. A power saw is dangerous; but I'm not going back thanks.

And nothing to say about the performance cost of these infallible tools? They
are not free.

~~~
unscaled
Right, except that Rust is a modern power saw with safety enclosure and C is a
rusted chainsaw that only works half of the time.

~~~
wepple
"only works half the time" is a bit of a stretch. what's your OS kernel
written in?

I agree we could do better than C. Everyone here talks about it every time
this happens, but does nothing. I'll take battle-hardened C over written-last-
week Rust, too. Especially crypto code.

------
chinathrow
Responsible disclosure, anyone?

~~~
detaro
The openSSL project committed the fix and description to their public repo 3
days ago (no clue if intentionally or not, but they didn't remove them). Not
much to hide anymore.

