
Heap overflow bug in OpenSSL - anthonyb
http://lists.grok.org.uk/pipermail/full-disclosure/2012-April/086585.html
======
djmdjm
FYI OpenSSH's sshd is not vulnerable, despite using OpenSSL. Back in 2002 and
after a different ASN.1 bug, Markus Friedl observed that RSA verification used
the OpenSSL's full ASN.1 parser to parse PKCS#1 RSA signatures despite them
having an almost entirely fixed format under the parameters used in the SSH
protocol.

He wrote a minimal RSA signature verification implementation that we've used
ever since. This is the eighth bug that it has saved us from in the last ten
years.

Attack surface reduction works.

Note, other parts of OpenSSH are vulnerable - specifically private key
loading. So, if you allow untrusted users to supply private keys to ssh, ssh-
add or ssh-keygen running in a privileged context, then you should patch this
bug ASAP.

~~~
agl
While having a minimal signature verifier is good, you wouldn't have been hit
by this bug in any case. This only affects parsing from a BIO or a FILE, and
the OpenSSL RSA code parses the signature from memory.

------
jws
ASN.1 is a ludicrously complicated format for what it does.

Its designers added all sorts of ways to save a few bits here and there by
creating optional special cases to be handled in the encoders and decoders.

This makes complicated code with lots of edge cases for bugs to hide in, Cisco
SNMP implementations were notoriously savaged by this. Ironically, it also
requires lots of branches which slow down modern processors more than just
reading the bits.

Fortunately most uses of ASN.1 have died. SNMP and SSL are still using it.
It's not 1984 anymore, let it die.

~~~
tptacek
Actually, all of the SNMP implementations were savaged by ASN.1/BER
implementation bugs: the format is so poorly understood that virtually all of
them derived from the same code.

There is a trick to ASN.1/DER encoding that gets it down into the hundreds-
of- lines- of- code range (you get the whole document in memory and
serialize/unserialize it backwards). I once wrote a program that converted
arbitrary ASN.1/DER/BER buffers into a shell script that regenerated the same.
It's not as complicated as it looks, but it's idiosyncratic and you have to
think about it a specific way.

That said: I agree, it should be scorched off the planet with fire.

~~~
samroberts
How do you go backwards? You don't know the length of the last
element/position of the last tag unless you work forward from the front, as
far as I can see.

BER is complicated, but no more complicated than rfc-822 style headers, which
are also poorly parsed by ad-hoc parsers, and were a wide source of
vulnerabilities.

------
dguido
Mark Dowd found this bug and described it in The Art of Software Security
Assessment (TAOSSA) in 2006. lawl.

<https://twitter.com/#!/mdowd/status/192986878138523648>

Here's the actual page: <http://i.imgur.com/vPjOR.jpg>

------
dhx
Sample emergency bump for Gentoo:

 __Firstly and most importantly: check[http://sources.gentoo.org/cgi-
bin/viewvc.cgi/gentoo-x86/dev-...](http://sources.gentoo.org/cgi-
bin/viewvc.cgi/gentoo-x86/dev-libs/openssl/ChangeLog?view=markup) to see
whether the Gentoo developers have already bumped OpenSSL in the official
repository. If so, ignore everything below! __

    
    
    wget -O /usr/portage/distfiles/openssl-1.0.0i.tar.gz http://www.openssl.org/source/openssl-1.0.0i.tar.gz
      chown portage:portage /usr/portage/distfiles/openssl-1.0.0i.tar.gz
      chmod g+w /usr/portage/distfiles/openssl-1.0.0i.tar.gz
      mkdir -p /usr/local/portage/dev-libs/openssl
      cp /usr/portage/dev-libs/openssl/openssl-1.0.0h.ebuild /usr/local/portage/dev-libs/openssl/openssl-1.0.0i.ebuild
      cp -R /usr/portage/dev-libs/openssl/files /usr/local/portage/dev-libs/openssl/
      ebuild /usr/local/portage/dev-libs/openssl/openssl-1.0.0i.ebuild digest
      emerge -1q =dev-libs/openssl-1.0.0i
      shutdown -r -t 0 now

Skip the first 3 commands when mirrors have the latest OpenSSL tarballs.

Preferably skip the last command and manually restart daemons that rely on
OpenSSL. I have used the drastic example of restarting the entire server in
case someone blindly follows the above commands without thinking it through
carefully.

Note that openssl-1.0.1* is currently masked in Gentoo ~amd64. If you have it
unmasked, it should be easy to adjust the above commands to use openssl-1.0.1a
instead.

------
agl
The OpenSSL advisory contains details of the problematic functions, which are
considerably smaller than the full set of ASN.1 functions:

<http://www.openssl.org/news/secadv_20120419.txt>

~~~
dhx
To get an idea of which packages may be affected (try different functions for
more results):

[https://github.com/search?q=d2i_X509_bio&type=Code](https://github.com/search?q=d2i_X509_bio&type=Code)

<http://www.koders.com/default.aspx?s=d2i_X509_bio>

Examples of impacted software include Android, Apache HTTPd (mod_ssl)[1] and
Ruby. To reiterate what you've already stated elsewhere in this discussion,
software shouldn't have a need to call these functions to validate
certificates provided by remote clients. Users of email clients making heavy
use of S/MIME and administrators managing PKI (signing, revoking, etc) may
need to apply caution.

[1] See line 99 at
[https://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/...](https://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c?view=markup)
where Apache tries to load a PEM formatted certificate. If this fails, Apache
tries loading the file as a DER+Base64 formatted certificate or as a last
resort, just DER (both which use the vulnerable d2i_X509_bio function). Given
that the PEM format is the standard that most Apache administrators are using
and injection of vulnerable certificates and keys usually requires root
permissions, Apache/mod_ssl users can probably treat this vulnerability as a
non-issue.

~~~
0x0
What about HTTP SSL Client Certificates; i.e. what if a "browser" (attacker)
simply includes a bad SSL Client Certificate in a request?

------
tptacek
This looks pretty bad. The canonical bit of untrusted ASN.1/DER information
most systems deal with: SSL certificates. In particular, all OpenSSL-based SSL
client software (Ruby and Python HTTP libraries), and web applications that
use client certificates for authentication.

When Tavis Ormandy says "textbook heap overflow", patch fast.

~~~
agl
Parsing from PEM isn't affected, nor is parsing of SSL certificates in SSL
because libssl doesn't use the BIO functions.

It's mostly users of OpenSSL who are calling d2i_XXX_bio or d2i_XXX_fp.

~~~
tptacek
Rats, you're right. I should stop commenting 15 minutes after I wake up.

------
dhx
Tests against some commonly used software that make use of libssl to load
certificates from disk (or perhaps a socket in strange circumstances):

php-5.4.1RC2 (and php-5.4.0): potentially affected _(one match for
d2i_PKCS12_bio)_

    
    
      tar -xjOf /usr/portage/distfiles/php-5.4.1RC2.tar.bz2 | grep d2i
    
    

nginx-1.1.19: probably safe _(1 match for d2i_SSL_SESSION)_

    
    
      tar -xzOf /usr/portage/distfiles/nginx-1.1.19.tar.gz | grep d2i
    
    

postgresql-9.1.3: probably safe _(no matches)_

    
    
      tar -xjOf /usr/portage/distfiles/postgresql-9.1.3.tar.bz2 | grep d2i
    
    

postfix-2.9.1: probably safe _(one match for X509_get_ext_d2i and
d2i_SSL_SESSION each)_

    
    
      tar -xzOf /usr/portage/distfiles/postfix-2.9.1.tar.gz | grep d2i
    
    

dovecot-2.1.4: probably safe _(3 matches for d2i_DHparams, 1 match for
X509_get_ext_d2i)_

    
    
      tar -xzOf /usr/portage/distfiles/dovecot-2.1.4.tar.gz | grep d2i

~~~
mhurron
In most cases these packages should be loading libssl as a shared library.
Simply updating libssl to a fixed version would solve the problem for all of
those instances.

------
__david__
The commit is here if you want to see what they did to fix it:
<http://cvs.openssl.org/chngview?cn=22447>

------
munin
apparently, this was published in 2006:
<https://twitter.com/mdowd/status/192986878138523648>

------
EvilTerran
Could this be used maliciously to, say, gain access or control of an affected
system, or to subvert certificate checking? Or is crashing the process that's
using OpenSSL the worst it could do?

~~~
tptacek
If you are using the affected ASN.1 functions and you are feeding them
attacker-controlled ASN.1 data (say, SSL certificates during user
configuration), it is likely that attackers will be able to run their own code
in your programs.

You are _probably_ not using the affected functions.

~~~
EvilTerran
Am I right in thinking that would involve taking advantage of the memory
corruption to inject code by using an appropriately constructed cert?

~~~
tptacek
Yes, but read 'agl's comments carefully: the systems impacted by this are
going to tend to be ones that do special-case configuration of SSL
certificates. We're not talking about browsers and (for the most part) web
servers here.

A hypothetical future Github feature that allowed users to upload SSL certs in
lieu of SSH keys might have to review their code to make sure they weren't
using OpenSSL BIOs to read certs from (or just patch).

You should patch anyways. From now on, professional security assessments are
going to doc this version of OpenSSL as a vulnerability.

~~~
EvilTerran
Oh, I'm not using OpenSSL professionally - I'm pretty much just a curious
amateur when it comes to computer security.

Good to know I've not got anything to worry about personally, though. You've
explained it well.

~~~
tptacek
Adam Langley's the one who did the real explaining on this thread; thank 'agl.
:|

------
dchest
9th vulnerability in 2012.

------
linker3000
Is this likely to have any implications for users of OpenVPN?

~~~
dhx
Yes.

wget -q -O -
[http://swupdate.openvpn.org/community/releases/openvpn-2.2.2...](http://swupdate.openvpn.org/community/releases/openvpn-2.2.2.tar.gz)
| tar -xzO | grep d2i

    
    
      if ((eku = (EXTENDED_KEY_USAGE *)X509_get_ext_d2i (x509, NID_ext_key_usage, NULL, NULL)) == NULL) {
      if ((ku = (ASN1_BIT_STRING *)X509_get_ext_d2i (x509, NID_key_usage, NULL, NULL)) == NULL) {
      p12 = d2i_PKCS12_bio(b64, NULL);
      p12 = d2i_PKCS12_fp(fp, NULL);
      cert = d2i_X509(NULL, (const unsigned char **) &cd->cert_context->pbCertEncoded,

~~~
js2
Strictly speaking, yes it is likely to have some implication(s).

I think more importantly is whether it is likely to allow a server to be
remotely exploited. The answer to that is "probably not", and "very likely
not" if using OpenVPN's tls-auth option. At least as far as I understand the
issue.

Also, <http://article.gmane.org/gmane.network.openvpn.devel/6309>

------
newlog
I suppose it could not derive to arbitrary code execution, isn't it?

------
adobriyan
Lengths should always be unsigned.

Always.

I'd even say they should always be uintmax_t in C.

~~~
djmdjm
Please explain how a string length would need more than size_t to represent
it.

~~~
adobriyan
This is largely theoretical but still.

Smart programming languages have bignum integers and seamless promotion
between fixnum and bignum versions. Thus they avoid the issue completely, no
stupid casts (especially casts which change both signedness and size), no
overflows, no nothing, just an integer length.

It's a perfect world and everyone should try to achieve it.

C is not such programming language, with the closest approximation being
uintmax_t.

size_t is of course enough in practice.

~~~
wladimir
Even for fixnums, integer overflows and underflows of any kind should ideally
result in an exception by default. I think it's a pity that C(++) doesn't have
support for this. A lot of bugs and weaknesses could have been prevented (for
example, CWE 680 <http://cwe.mitre.org/data/definitions/680.html>).

I know this decision (to simply wrap around in the case of an over/underflow)
was probably performance-driven, but on the other hand, if the common
languages had required it, CPUs would have better support for it...

Edit: Some googling shows that Microsoft has a SafeInt class in common use
that reports under/overflow: <http://safeint.codeplex.com/> . Still it feels
like a kludge for this to be not part of the main language.

~~~
JoachimSchipper
(Hint: gcc has a -ftrapv option, which traps on signed integer overflow.
Unsigned overflow _is_ defined, so trapping on that would break working code.)

~~~
wladimir
I don't think I'd necessarily want to trap on all (signed) integer overflows.
As you say, it might break working code. There's just too much C/C++ code
around to change that retroactively. And for modular arithmetic and such it is
desirable for integers to wrap around.

But a "trap on overflow" signed and unsigned int type would be nice.

~~~
JoachimSchipper
Recent gcc versions use the fact that signed overflow is undefined to do some
unexpected optimizations (in particular, a + b < a will never be true if a, b
are ints.) I don't think -ftrapv is going to cause many additional errors, but
I haven't actually tried it. (Also, <http://embed.cs.utah.edu/ioc/> looks
interesting.)

~~~
Natsu
> (in particular, a + b < a will never be true if a, b are ints.)

Code of exactly that form in the patch for this bug made me do a double take.
Fortunately, they'd also changed the a & b from plain ints to size_t, so it
was ok.

------
sohn
Cool Debian will have it in 8 years

~~~
comice
they already released a fix for it, yesterday.

