List Info

Thread: GnuPG: remotely controllable function pointer




GnuPG: remotely controllable function pointer
user name
2006-12-06 15:55:52
     GnuPG: remotely controllable function pointer
[CVE-2006-6235]
   
============================================================
===
                              2006-12-04

Summary
=======

Tavis Ormandy of the Gentoo security team identified a
severe and
exploitable bug in the processing of encrypted packets in
GnuPG.

[ Please do not send private mail in response to this
message.  The
  mailing list gnupg-devel is the best place to discuss this
problem
  (please subscribe first so you don't need moderator
approval [1]). ]


Impact
======

Using malformed OpenPGP packets an attacker is able to
modify and
dereference a function pointer in GnuPG.  This is a remotely
exploitable bug and affects any use of GnuPG where an
attacker can
control the data processed by GnuPG.  It is not necessary
limited to
encrypted data, also signed data may be affected.

Affected versions: All versions of GnuPG   < 1.4.6 
                   All versions of GnuPG-2 < 2.0.2
                   All beta versions of GnuPG-2 (1.9.0 ..
1.9.95)
Affected tools: gpg, gpgv, gpg2 and gpgv2.
Affected platforms: All.

gpg-agent, gpgsm as well as other tools are not affected.

A workaround is not known. 


Solution
========

If you are using a vendor supplied version of GnuPG:

 * Wait for an update from your vendor.  Vendors have been
informed on
   Saturday December 2, less than a day after this bug has
been reported.

If you are using GnuPG 1.4: 

 * Update as soon as possible to GnuPG 1.4.6. It has been
uploaded to
   the usual location: ftp://ftp.gnupg.org/gcrypt/gnupg/. 
This version
   was due to be released anyway this week.  See
   http://www.gnupg.org/d
ownload/ for details.

 * Or: As another and less intrusive option, apply the
attached patch
   to GnuPG 1.4.5.  This is the smallest possible fix.

If you are using GnuPG 2.0:

 * Apply the attached patch against GnuPG 2.0.1.

 * Or: Stop using gpg2 and gpgv2, install GnuPG 1.4.6 and
use gpg and gpgv
   instead.

If you are using a binary Windows version of GnuPG:

 * A binary version of GnuPG 1.4.6 for Windows is available
as usual.

 * Gpg4win 1.0.8, including GnuPG 1.4.6, is available. 
Please go to
   http://www.gpg4win.org .




Background
==========

GnuPG uses data structures called filters to process OpenPGP
messages.
These filters ware used in a similar way as a pipelines in
the shell.
For communication between these filters context structures
are used.
These are usually allocated on the stack and passed to the
filter
functions.  At most places the OpenPGP data stream fed into
these
filters is closed before the context structure gets
deallocated.
While decrypting encrypted packets, this may not happen in
all cases
and the filter may use a void contest structure filled with
garbage.
An attacker may control this garbage.  The filter context
includes
another context used by the low-level decryption to access
the
decryption algorithm.  This is done using a function
pointer.  By
carefully crafting an OpenPGP message, an attacker may
control this
function pointer and call an arbitrary function of the
process.
Obviously an exploit needs to prepared for a specific
version,
compiler, libc, etc to be successful - but it is definitely
doable.

Fixing this is obvious: We need to allocate the context on
the heap
and use a reference count to keep it valid as long as either
the
controlling code or the filter code needs it.

We have checked all other usages of such a stack based
filter contexts
but fortunately found no other vulnerable places.  This
allows to
release a relatively small patch.  However, for reasons of
code
cleanness and easier audits we will soon start to change all
these
stack based filter contexts to heap based ones.


Support 
=======

g10 Code GmbH, a Duesseldorf based company owned and headed
by GnuPG's
principal author, is currently funding GnuPG development. 
As evident
by the two vulnerabilities found within a week, a review of
the entire
code base should be undertaken as soon as possible.  As
maintainers we
try to do our best and are working slowly through the code. 
The long
standing plan is to scrutinize the 2.0 code base, write more
test
cases and to backport new fixes and cleanups to 1.4. 
However, as a
small company our resources are limited and we need to
prioritize
other projects which get us actual revenues.  Support
contracts or
other financial backing would greatly help us to improve the
quality
of GnuPG.


Thanks
======

Tavis Ormandy found this vulnerability.




[1] See h
ttp://lists.gnupg.org/mailman/listinfo/gnupg-devel .


-- 
g10 Code GmbH       http://g10code.com     
AmtsGer. Wuppertal HRB 14459
Hüttenstr. 61                              
Geschäftsführung Werner Koch
D-40699 Erkrath  -=- The GnuPG Experts -=-  USt-Id
DE215605608
This is a patch against GnuPG 1.4.5.  Change the directory
to g10/ and
apply this patch.

2006-12-02  Werner Koch  <wkg10code.com>

	* encr-data.c: Allocate DFX context on the heap and not on
the
	stack.  Changes at several places.  Fixes CVE-2006-6235.
	


--- encr-data.c.orig	2006-05-16 14:34:26.000000000 +0200
+++ encr-data.c	2006-12-04 11:58:53.000000000 +0100
 -44,7
+44,27  typedef struct {
     char defer[20];
     int  defer_filled;
     int  eof_seen;
-} decode_filter_ctx_t;
+    int  refcount;
+} *decode_filter_ctx_t;
+
+
+/* Helper to release the decode context.  */
+static void
+release_dfx_context (decode_filter_ctx_t dfx)
+{
+  if (!dfx)
+    return;
+
+  assert (dfx->refcount);
+  if ( !--dfx->refcount )
+    {
+      cipher_close (dfx->cipher_hd);
+      dfx->cipher_hd = NULL;
+      md_close (dfx->mdc_hash);
+      dfx->mdc_hash = NULL;
+      xfree (dfx);
+    }
+}
 
 
 /****************
 -60,7
+80,10  decrypt_data( void *procctx, PKT_encrypt
     unsigned blocksize;
     unsigned nprefix;
 
-    memset( &dfx, 0, sizeof dfx );
+
+    dfx = xcalloc (1, sizeof *dfx);
+    dfx->refcount = 1;
+
     if( opt.verbose && !dek->algo_info_printed )
{
 	const char *s = cipher_algo_to_string( dek->algo );
 	if( s )
 -79,15
+102,15  decrypt_data( void *procctx, PKT_encrypt
 	BUG();
 
     if( ed->mdc_method ) {
-	dfx.mdc_hash = md_open( ed->mdc_method, 0 );
+	dfx->mdc_hash = md_open ( ed->mdc_method, 0 );
 	if ( DBG_HASHING )
-	    md_start_debug(dfx.mdc_hash, "checkmdc");
+	    md_start_debug (dfx->mdc_hash,
"checkmdc");
     }
-    dfx.cipher_hd = cipher_open( dek->algo,
-				 ed->mdc_method? CIPHER_MODE_CFB
-					       : CIPHER_MODE_AUTO_CFB, 1 );
+    dfx->cipher_hd = cipher_open ( dek->algo,
+                                   ed->mdc_method?
CIPHER_MODE_CFB
+                                                 :
CIPHER_MODE_AUTO_CFB, 1 );
     /* log_hexdump( "thekey", dek->key,
dek->keylen );*/
-    rc = cipher_setkey( dfx.cipher_hd, dek->key,
dek->keylen );
+    rc = cipher_setkey ( dfx->cipher_hd, dek->key,
dek->keylen );
     if( rc == G10ERR_WEAK_KEY )
       {
 	log_info(_("WARNING: message was encrypted with"
 -105,7
+128,7  decrypt_data( void *procctx, PKT_encrypt
         goto leave;
     }
 
-    cipher_setiv( dfx.cipher_hd, NULL, 0 );
+    cipher_setiv ( dfx->cipher_hd, NULL, 0 );
 
     if( ed->len ) {
 	for(i=0; i < (nprefix+2) && ed->len; i++,
ed->len-- ) {
 -122,8
+145,8  decrypt_data( void *procctx, PKT_encrypt
 	    else
 		temp[i] = c;
     }
-    cipher_decrypt( dfx.cipher_hd, temp, temp, nprefix+2);
-    cipher_sync( dfx.cipher_hd );
+    cipher_decrypt ( dfx->cipher_hd, temp, temp,
nprefix+2);
+    cipher_sync ( dfx->cipher_hd );
     p = temp;
 /* log_hexdump( "prefix", temp, nprefix+2 ); */
     if(dek->symmetric
 -133,34
+156,34  decrypt_data( void *procctx, PKT_encrypt
 	goto leave;
       }
 
-    if( dfx.mdc_hash )
-	md_write( dfx.mdc_hash, temp, nprefix+2 );
+    if ( dfx->mdc_hash )
+	md_write ( dfx->mdc_hash, temp, nprefix+2 );
 
-    if( ed->mdc_method )
-	iobuf_push_filter( ed->buf, mdc_decode_filter, &dfx
);
+    dfx->refcount++;
+    if ( ed->mdc_method )
+	iobuf_push_filter( ed->buf, mdc_decode_filter, dfx );
     else
-	iobuf_push_filter( ed->buf, decode_filter, &dfx );
+	iobuf_push_filter( ed->buf, decode_filter, dfx );
 
     proc_packets( procctx, ed->buf );
     ed->buf = NULL;
-    if( ed->mdc_method && dfx.eof_seen == 2 )
+    if( ed->mdc_method && dfx->eof_seen == 2
)
 	rc = G10ERR_INVALID_PACKET;
     else if( ed->mdc_method ) { /* check the mdc */
 	int datalen = md_digest_length( ed->mdc_method );
 
-	cipher_decrypt( dfx.cipher_hd, dfx.defer, dfx.defer, 20);
-	md_final( dfx.mdc_hash );
+	cipher_decrypt ( dfx->cipher_hd, dfx->defer,
dfx->defer, 20);
+	md_final ( dfx->mdc_hash );
 	if( datalen != 20
-	    || memcmp(md_read( dfx.mdc_hash, 0 ), dfx.defer,
datalen) )
+	    || memcmp(md_read( dfx->mdc_hash, 0 ),
dfx->defer, datalen) )
 	    rc = G10ERR_BAD_SIGN;
-	/*log_hexdump("MDC calculated:", md_read(
dfx.mdc_hash, 0), datalen);*/
-	/*log_hexdump("MDC message   :", dfx.defer,
20);*/
+	/*log_hexdump("MDC calculated:",md_read(
dfx->mdc_hash, 0), datalen);*/
+	/*log_hexdump("MDC message   :", dfx->defer,
20);*/
     }
     
 
   leave:
-    cipher_close(dfx.cipher_hd);
-    md_close( dfx.mdc_hash );
+    release_dfx_context (dfx);
     return rc;
 }
 
 -171,7
+194,7  static int
 mdc_decode_filter( void *opaque, int control, IOBUF a,
 					      byte *buf, size_t *ret_len)
 {
-    decode_filter_ctx_t *dfx = opaque;
+    decode_filter_ctx_t dfx = opaque;
     size_t n, size = *ret_len;
     int rc = 0;
     int c;
 -226,8
+249,10  mdc_decode_filter( void *opaque, int con
 	}
 
 	if( n ) {
-	    cipher_decrypt( dfx->cipher_hd, buf, buf, n);
-	    md_write( dfx->mdc_hash, buf, n );
+            if (dfx->cipher_hd)
+                cipher_decrypt( dfx->cipher_hd, buf,
buf, n);
+            if (dfx->mdc_hash)
+                md_write( dfx->mdc_hash, buf, n );
 	}
 	else {
 	    assert( dfx->eof_seen );
 -235,6
+260,9  mdc_decode_filter( void *opaque, int con
 	}
 	*ret_len = n;
     }
+    else if ( control == IOBUFCTRL_FREE ) {
+        release_dfx_context (dfx);
+    }
     else if( control == IOBUFCTRL_DESC ) {
 	*(char**)buf = "mdc_decode_filter";
     }
 -244,7
+272,7  mdc_decode_filter( void *opaque, int con
 static int
 decode_filter( void *opaque, int control, IOBUF a, byte
*buf, size_t *ret_len)
 {
-    decode_filter_ctx_t *fc = opaque;
+    decode_filter_ctx_t fc = opaque;
     size_t n, size = *ret_len;
     int rc = 0;
 
 -252,12
+280,17  decode_filter( void *opaque, int control
 	assert(a);
 	n = iobuf_read( a, buf, size );
 	if( n == -1 ) n = 0;
-	if( n )
-	    cipher_decrypt( fc->cipher_hd, buf, buf, n);
+	if( n ) {
+            if (fc->cipher_hd)
+                cipher_decrypt( fc->cipher_hd, buf, buf,
n);
+        }
 	else
 	    rc = -1; /* eof */
 	*ret_len = n;
     }
+    else if ( control == IOBUFCTRL_FREE ) {
+        release_dfx_context (fc);
+    }
     else if( control == IOBUFCTRL_DESC ) {
 	*(char**)buf = "decode_filter";
     }
This is a patch against GnuPG 2.0.1. Change the directory to
g10/ and
apply this patch.

2006-12-02  Werner Koch  <wkg10code.com>

	* encr-data.c: Allocate DFX context on the heap and not on
the
	stack.  Changes at several places.  Fixes CVE-2006-6235.
	

Index: encr-data.c
============================================================
=======
--- encr-data.c	(revision 4352)
+++ encr-data.c	(working copy)
 -39,16
+39,37 
 static int decode_filter ( void *opaque, int control, IOBUF
a,
 					byte *buf, size_t *ret_len);
 
-typedef struct 
+typedef struct decode_filter_context_s
 {
   gcry_cipher_hd_t cipher_hd;
   gcry_md_hd_t mdc_hash;
   char defer[22];
   int  defer_filled;
   int  eof_seen;
-} decode_filter_ctx_t;
+  int  refcount;
+} *decode_filter_ctx_t;
 
 
+/* Helper to release the decode context.  */
+static void
+release_dfx_context (decode_filter_ctx_t dfx)
+{
+  if (!dfx)
+    return;
+
+  assert (dfx->refcount);
+  if ( !--dfx->refcount )
+    {
+      gcry_cipher_close (dfx->cipher_hd);
+      dfx->cipher_hd = NULL;
+      gcry_md_close (dfx->mdc_hash);
+      dfx->mdc_hash = NULL;
+      xfree (dfx);
+    }
+}
+
+
+
 /****************
  * Decrypt the data, specified by ED with the key DEK.
  */
 -62,7
+83,11 
   unsigned blocksize;
   unsigned nprefix;
   
-  memset( &dfx, 0, sizeof dfx );
+  dfx = xtrycalloc (1, sizeof *dfx);
+  if (!dfx)
+    return gpg_error_from_syserror ();
+  dfx->refcount = 1;
+
   if ( opt.verbose && !dek->algo_info_printed )
     {
       const char *s = gcry_cipher_algo_name (dek->algo);
 -77,20
+102,20 
     goto leave;
   blocksize = gcry_cipher_get_algo_blklen (dek->algo);
   if ( !blocksize || blocksize > 16 )
-    log_fatal("unsupported blocksize %un",
blocksize );
+    log_fatal ("unsupported blocksize %un",
blocksize );
   nprefix = blocksize;
   if ( ed->len && ed->len < (nprefix+2) )
     BUG();
 
   if ( ed->mdc_method ) 
     {
-      if (gcry_md_open (&dfx.mdc_hash,
ed->mdc_method, 0 ))
+      if (gcry_md_open (&dfx->mdc_hash,
ed->mdc_method, 0 ))
         BUG ();
       if ( DBG_HASHING )
-        gcry_md_start_debug (dfx.mdc_hash,
"checkmdc");
+        gcry_md_start_debug (dfx->mdc_hash,
"checkmdc");
     }
 
-  rc = gcry_cipher_open (&dfx.cipher_hd, dek->algo,
+  rc = gcry_cipher_open (&dfx->cipher_hd,
dek->algo,
                          GCRY_CIPHER_MODE_CFB,
                          (GCRY_CIPHER_SECURE
                           | ((ed->mdc_method ||
dek->algo >= 100)?
 -104,7
+129,7 
 
 
   /* log_hexdump( "thekey", dek->key,
dek->keylen );*/
-  rc = gcry_cipher_setkey (dfx.cipher_hd, dek->key,
dek->keylen);
+  rc = gcry_cipher_setkey (dfx->cipher_hd, dek->key,
dek->keylen);
   if ( gpg_err_code (rc) == GPG_ERR_WEAK_KEY )
     {
       log_info(_("WARNING: message was encrypted
with"
 -123,7
+148,7 
       goto leave;
     }
 
-  gcry_cipher_setiv (dfx.cipher_hd, NULL, 0);
+  gcry_cipher_setiv (dfx->cipher_hd, NULL, 0);
 
   if ( ed->len )
     {
 -144,8
+169,8 
           temp[i] = c;
     }
   
-  gcry_cipher_decrypt (dfx.cipher_hd, temp, nprefix+2,
NULL, 0);
-  gcry_cipher_sync (dfx.cipher_hd);
+  gcry_cipher_decrypt (dfx->cipher_hd, temp, nprefix+2,
NULL, 0);
+  gcry_cipher_sync (dfx->cipher_hd);
   p = temp;
   /* log_hexdump( "prefix", temp, nprefix+2 ); */
   if (dek->symmetric
 -155,17
+180,18 
       goto leave;
     }
   
-  if ( dfx.mdc_hash )
-    gcry_md_write (dfx.mdc_hash, temp, nprefix+2);
-  
+  if ( dfx->mdc_hash )
+    gcry_md_write (dfx->mdc_hash, temp, nprefix+2);
+
+  dfx->refcount++;
   if ( ed->mdc_method )
-    iobuf_push_filter( ed->buf, mdc_decode_filter,
&dfx );
+    iobuf_push_filter ( ed->buf, mdc_decode_filter, dfx
);
   else
-    iobuf_push_filter( ed->buf, decode_filter, &dfx
);
+    iobuf_push_filter ( ed->buf, decode_filter, dfx );
 
   proc_packets ( procctx, ed->buf );
   ed->buf = NULL;
-  if ( ed->mdc_method && dfx.eof_seen == 2 )
+  if ( ed->mdc_method && dfx->eof_seen == 2 )
     rc = gpg_error (GPG_ERR_INV_PACKET);
   else if ( ed->mdc_method )
     { 
 -184,26
+210,28 
          bytes are appended.  */
       int datalen = gcry_md_get_algo_dlen
(ed->mdc_method);
 
-      gcry_cipher_decrypt (dfx.cipher_hd, dfx.defer, 22,
NULL, 0);
-      gcry_md_write (dfx.mdc_hash, dfx.defer, 2);
-      gcry_md_final (dfx.mdc_hash);
+      assert (dfx->cipher_hd);
+      assert (dfx->mdc_hash);
+      gcry_cipher_decrypt (dfx->cipher_hd,
dfx->defer, 22, NULL, 0);
+      gcry_md_write (dfx->mdc_hash, dfx->defer, 2);
+      gcry_md_final (dfx->mdc_hash);
 
-      if (dfx.defer[0] != 'xd3' || dfx.defer[1] != 'x14'
)
+      if (dfx->defer[0] != 'xd3' || dfx->defer[1] !=
'x14' )
         {
           log_error("mdc_packet with invalid
encodingn");
           rc = gpg_error (GPG_ERR_INV_PACKET);
         }
       else if (datalen != 20
-               || memcmp (gcry_md_read (dfx.mdc_hash,
0),dfx.defer+2,datalen))
+               || memcmp (gcry_md_read (dfx->mdc_hash,
0),
+                          dfx->defer+2,datalen ))
         rc = gpg_error (GPG_ERR_BAD_SIGNATURE);
-      /* log_printhex("MDC message:", dfx.defer,
22); */
-      /* log_printhex("MDC calc:", gcry_md_read
(dfx.mdc_hash,0), datalen); */
+      /* log_printhex("MDC message:",
dfx->defer, 22); */
+      /* log_printhex("MDC calc:", gcry_md_read
(dfx->mdc_hash,0), datalen); */
     }
   
   
  leave:
-  gcry_cipher_close (dfx.cipher_hd);
-  gcry_md_close (dfx.mdc_hash);
+  release_dfx_context (dfx);
   return rc;
 }
 
 -214,7
+242,7 
 mdc_decode_filter (void *opaque, int control, IOBUF a,
                    byte *buf, size_t *ret_len)
 {
-  decode_filter_ctx_t *dfx = opaque;
+  decode_filter_ctx_t dfx = opaque;
   size_t n, size = *ret_len;
   int rc = 0;
   int c;
 -226,11
+254,11 
     }
   else if( control == IOBUFCTRL_UNDERFLOW )
     {
-      assert(a);
-      assert( size > 44 );
+      assert (a);
+      assert ( size > 44 );
       
       /* Get at least 22 bytes and put it somewhere ahead
in the buffer. */
-      for(n=22; n < 44 ; n++ )
+      for (n=22; n < 44 ; n++ )
         {
           if( (c = iobuf_get(a)) == -1 )
             break;
 -279,8
+307,10 
 
       if ( n )
         {
-          gcry_cipher_decrypt (dfx->cipher_hd, buf, n,
NULL, 0);
-          gcry_md_write (dfx->mdc_hash, buf, n);
+          if ( dfx->cipher_hd )
+            gcry_cipher_decrypt (dfx->cipher_hd, buf, n,
NULL, 0);
+          if ( dfx->mdc_hash )
+            gcry_md_write (dfx->mdc_hash, buf, n);
 	}
       else
         {
 -289,6
+319,10 
 	}
       *ret_len = n;
     }
+  else if ( control == IOBUFCTRL_FREE ) 
+    {
+      release_dfx_context (dfx);
+    }
   else if ( control == IOBUFCTRL_DESC ) 
     {
       *(char**)buf = "mdc_decode_filter";
 -300,7
+334,7 
 static int
 decode_filter( void *opaque, int control, IOBUF a, byte
*buf, size_t *ret_len)
 {
-  decode_filter_ctx_t *fc = opaque;
+  decode_filter_ctx_t fc = opaque;
   size_t n, size = *ret_len;
   int rc = 0;
   
 -311,11
+345,18 
       if ( n == -1 )
         n = 0;
       if ( n )
-        gcry_cipher_decrypt (fc->cipher_hd, buf, n,
NULL, 0);
+        {
+          if (fc->cipher_hd)
+            gcry_cipher_decrypt (fc->cipher_hd, buf, n,
NULL, 0);
+        }
       else
         rc = -1; /* EOF */
       *ret_len = n;
     }
+  else if ( control == IOBUFCTRL_FREE ) 
+    {
+      release_dfx_context (fc);
+    }
   else if ( control == IOBUFCTRL_DESC )
     {
       *(char**)buf = "decode_filter";
_______________________________________________
Gnupg-announce mailing list
Gnupg-announcegnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-announce

[1]

about | contact  Other archives ( Real Estate discussion Medical topics )