Versions affected: 0.9.8*, 0.9.9*
When the SSL enc_read_ctx and enc_write_ctx are allocated they are not
immediately initialized and if an error occurs between their allocation
and intialization the resulting SSL structure will contain bogus values
in them, resulting in a crash when ssl_clear_cipher_ctx() is called.
For example in tls1_change_cipher, s->enc_read_ctx is allocated on line
268 and intialized on line 334 (openssl 0.9.8e, ssl/t1_enc.c). In
between there are several error conditions (compression library or
malloc errors) that can result in a "goto err" and an immediate exit,
leaving s->enc_read_ctx or s->enc_write_ctx uninitialized. A future call
to ssl_clear_cipher_ctx (triggered by a SSL_free()) will result in
a crash in this case.
The problem appears in: tls1_change_cipher_state(),
ssl3_change_cipher_state() and ssl2_enc_init().
I've stumbled across this bug in low memory condition, when mallocs
started to return 0. In my case the zlib compression wanted to allocate
larger chunks of memory (65k), but failed causing COMP_CTX_new to fail
in tls1_change_cipher() and the bug described above.
The attached patch moves the intialization immediately after allocation
so that exiting is "safe". An alternative approach would be to free and
nullify the enc_read/write_ctx on error (before goto err).
Special tls1_change_cipher_state note: I've also removed a few lines of
duplicate enc_write_ctx alloc code (harmless, see the patch for more
info).
Patch url for easier viewing:
http://www.iptel.org/~andrei/openssl_patches/enc_read_write_ctx_init_immediately.patch
diff -rup openssl-0.9.8-stable-SNAP-20070212.orig/ssl/s2_enc.c
openssl-0.9.8-stable-SNAP-20070212/ssl/s2_enc.c
--- openssl-0.9.8-stable-SNAP-20070212.orig/ssl/s2_enc.c 2003-12-27
17:10:30.000000000 +0100
+++ openssl-0.9.8-stable-SNAP-20070212/ssl/s2_enc.c 2007-02-14
11:58:23.000000000 +0100
@@ -82,15 +82,18 @@ int ssl2_enc_init(SSL *s, int client)
((s->enc_read_ctx=(EVP_CIPHER_CTX *)
OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL))
goto err;
+
+ /* make sure it's intialized in case the malloc for enc_write_ctx fails
+ * and we exit with an error */
+ rs= s->enc_read_ctx;
+ EVP_CIPHER_CTX_init(rs);
+
if ((s->enc_write_ctx == NULL) &&
((s->enc_write_ctx=(EVP_CIPHER_CTX *)
OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL))
goto err;
- rs= s->enc_read_ctx;
ws= s->enc_write_ctx;
-
- EVP_CIPHER_CTX_init(rs);
EVP_CIPHER_CTX_init(ws);
num=c->key_len;
diff -rup openssl-0.9.8-stable-SNAP-20070212.orig/ssl/s3_enc.c
openssl-0.9.8-stable-SNAP-20070212/ssl/s3_enc.c
--- openssl-0.9.8-stable-SNAP-20070212.orig/ssl/s3_enc.c 2005-10-01
03:07:28.000000000 +0200
+++ openssl-0.9.8-stable-SNAP-20070212/ssl/s3_enc.c 2007-02-14
11:54:30.000000000 +0100
@@ -221,6 +221,9 @@ int ssl3_change_cipher_state(SSL *s, int
reuse_dd = 1;
else if
((s->enc_read_ctx=OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL)
goto err;
+ else
+ /* make sure it's intialized in case we exit later with
an error */
+ EVP_CIPHER_CTX_init(s->enc_read_ctx);
dd= s->enc_read_ctx;
s->read_hash=m;
#ifndef OPENSSL_NO_COMP
@@ -254,6 +257,9 @@ int ssl3_change_cipher_state(SSL *s, int
reuse_dd = 1;
else if
((s->enc_write_ctx=OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL)
goto err;
+ else
+ /* make sure it's intialized in case we exit later with
an error */
+ EVP_CIPHER_CTX_init(s->enc_write_ctx);
dd= s->enc_write_ctx;
s->write_hash=m;
#ifndef OPENSSL_NO_COMP
@@ -279,7 +285,6 @@ int ssl3_change_cipher_state(SSL *s, int
if (reuse_dd)
EVP_CIPHER_CTX_cleanup(dd);
- EVP_CIPHER_CTX_init(dd);
p=s->s3->tmp.key_block;
i=EVP_MD_size(m);
diff -rup openssl-0.9.8-stable-SNAP-20070212.orig/ssl/t1_enc.c
openssl-0.9.8-stable-SNAP-20070212/ssl/t1_enc.c
--- openssl-0.9.8-stable-SNAP-20070212.orig/ssl/t1_enc.c 2006-05-07
15:07:44.000000000 +0200
+++ openssl-0.9.8-stable-SNAP-20070212/ssl/t1_enc.c 2007-02-13
14:05:02.000000000 +0100
@@ -267,6 +267,9 @@ int tls1_change_cipher_state(SSL *s, int
reuse_dd = 1;
else if
((s->enc_read_ctx=OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL)
goto err;
+ else
+ /* make sure it's intialized in case we exit later with
an error */
+ EVP_CIPHER_CTX_init(s->enc_read_ctx);
dd= s->enc_read_ctx;
s->read_hash=m;
#ifndef OPENSSL_NO_COMP
@@ -301,10 +304,9 @@ int tls1_change_cipher_state(SSL *s, int
reuse_dd = 1;
else if
((s->enc_write_ctx=OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL)
goto err;
- if ((s->enc_write_ctx == NULL) &&
- ((s->enc_write_ctx=(EVP_CIPHER_CTX *)
- OPENSSL_malloc(sizeof(EVP_CIPHER_CTX))) == NULL))
- goto err;
+ else
+ /* make sure it's intialized in case we exit later with
an error */
+ EVP_CIPHER_CTX_init(s->enc_write_ctx);
dd= s->enc_write_ctx;
s->write_hash=m;
#ifndef OPENSSL_NO_COMP
@@ -331,7 +333,6 @@ int tls1_change_cipher_state(SSL *s, int
if (reuse_dd)
EVP_CIPHER_CTX_cleanup(dd);
- EVP_CIPHER_CTX_init(dd);
p=s->s3->tmp.key_block;
i=EVP_MD_size(m);
