[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] [PATCH] memory consumption (DoS) in openssl CVE-2009-4355
From:       "Michael K. Johnson" <johnsonm () rpath ! com>
Date:       2010-01-13 15:15:48
Message-ID: 20100113151548.GA19602 () logo ! rdu ! rpath ! com
[Download RAW message or body]

Previously, an initialization-related memory leak involving openssl
was given CVE-2008-1678 and worked around in mod_ssl; see for example
https://bugzilla.redhat.com/show_bug.cgi?id=447268
https://issues.apache.org/bugzilla/show_bug.cgi?id=44975
https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/224945
http://svn.apache.org/viewvc?view=rev&revision=654119

However, this did not resolve the general problem, and an rPath
customer recently reproduced essentially the same memory leak via
another pathway.  This new pathway was assigned CVE-2009-4355.
Initially, the suggestion was to fix the leak via modifications
to php or curl in the same way that mod_ssl was previously fixed,
but then Andy Grimm provided a patch to openssl that would not only
resolve the issue for curl/php but also for any other as-yet-unknown
new vectors.  Dr. Stephen Henson, an openssl core team member,
provided a new openssl patch which rPath has confirmed resolves
the issue, and which Dr. Henson is committing to upstream openssl.
Dr. Henson's patch is attached to this email.

The specific symptom of this new pathway is that any vulnerable
system will leak hundreds of KB of memory per SSLv3 connection after
apache has been gracefully restarted (SIGHUP).  Temporary mitigation
strategies include limiting the number of requests that an apache
worker can serve to limit the quantity of leaked memory, and doing
full restarts rather than graceful restarts of apache.

Some discussion regarding this issue is in two issue reports:
https://issues.rpath.com/browse/RPL-3157
https://bugzilla.redhat.com/show_bug.cgi?id=546707

(I cannot make the Red Hat bugzilla report public, but assume
that it will be made public today.)

["CVE-2009-4355.patch" (text/plain)]

From: "Dr. Stephen Henson" <steve@openssl.org>
To: Andy Grimm <agrimm@rpath.com>
Cc: Stefan Fritsch <sf@debian.org>, Kees Cook <kees@ubuntu.com>,
        Pierre Joye <pierre.php@gmail.com>,
        "Steven M. Christey" <coley@linus.mitre.org>,
        Daniel Stenberg <daniel@haxx.se>, curl-security@haxx.se,
        secalert@redhat.com, security@debian.org, security@suse.de,
        security@ubuntu.com, Joe Orton <jorton@redhat.com>, security@php.net,
        "Michael K. Johnson" <johnsonm@rpath.com>,
        Kurt Roeckx <kurt@roeckx.be>
Subject: Re: [PATCH] memory consumption (DoS) vulnerability involving libcurl
Message-ID: <20100111011852.GA14391@openssl.org>

...

I've attached a patch which uses an alternative technique. The main problem is
that the ex_data free function pointer is removed when
CRYPTO_cleanup_all_ex_data() is called. If the compression structure is
cleaned up directly this problem is avoided:

Index: crypto/comp/c_zlib.c
===================================================================
RCS file: /v/openssl/cvs/openssl/crypto/comp/c_zlib.c,v
retrieving revision 1.22
diff -u -r1.22 c_zlib.c
--- crypto/comp/c_zlib.c	13 Dec 2008 17:19:40 -0000	1.22
+++ crypto/comp/c_zlib.c	8 Jan 2010 23:56:13 -0000
@@ -136,15 +136,6 @@
 
 static int zlib_stateful_ex_idx = -1;
 
-static void zlib_stateful_free_ex_data(void *obj, void *item,
-	CRYPTO_EX_DATA *ad, int ind,long argl, void *argp)
-	{
-	struct zlib_state *state = (struct zlib_state *)item;
-	inflateEnd(&state->istream);
-	deflateEnd(&state->ostream);
-	OPENSSL_free(state);
-	}
-
 static int zlib_stateful_init(COMP_CTX *ctx)
 	{
 	int err;
@@ -188,6 +179,12 @@
 
 static void zlib_stateful_finish(COMP_CTX *ctx)
 	{
+	struct zlib_state *state =
+		(struct zlib_state *)CRYPTO_get_ex_data(&ctx->ex_data,
+			zlib_stateful_ex_idx);
+	inflateEnd(&state->istream);
+	deflateEnd(&state->ostream);
+	OPENSSL_free(state);
 	CRYPTO_free_ex_data(CRYPTO_EX_INDEX_COMP,ctx,&ctx->ex_data);
 	}
 
@@ -402,7 +399,7 @@
 			if (zlib_stateful_ex_idx == -1)
 				zlib_stateful_ex_idx =
 					CRYPTO_get_ex_new_index(CRYPTO_EX_INDEX_COMP,
-						0,NULL,NULL,NULL,zlib_stateful_free_ex_data);
+						0,NULL,NULL,NULL,NULL);
 			CRYPTO_w_unlock(CRYPTO_LOCK_COMP);
 			if (zlib_stateful_ex_idx == -1)
 				goto err;

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org



[prev in list] [next in list] [prev in thread] [next in thread]