From d90a1d23404f482cc4a5a2b2ee0f296d67ff2227 Mon Sep 17 00:00:00 2001
From: Werner Koch <wk@gnupg.org>
Date: Wed, 2 Oct 2013 09:11:43 +0200
Subject: [PATCH 1/2] gpg: Fix bug with deeply nested compressed packets.

* g10/mainproc.c (MAX_NESTING_DEPTH): New.
(proc_compressed): Return an error code.
(check_nesting): New.
(do_proc_packets): Check packet nesting depth.  Handle errors from
check_compressed.

Signed-off-by: Werner Koch <wk@gnupg.org>
---
 NEWS           |    3 +++
 g10/mainproc.c |   52 +++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 46 insertions(+), 9 deletions(-)

--- a/g10/mainproc.c
+++ b/g10/mainproc.c
@@ -1,6 +1,7 @@
 /* mainproc.c - handle packets
  * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007,
  *               2008 Free Software Foundation, Inc.
+ * Copyright (C) 2013 Werner Koch
  *
  * This file is part of GnuPG.
  *
@@ -41,6 +42,11 @@
 #include "photoid.h"
 
 
+/* Put an upper limit on nested packets.  The 32 is an arbitrary
+   value, a much lower should actually be sufficient.  */
+#define MAX_NESTING_DEPTH 32
+
+
 struct kidlist_item {
     struct kidlist_item *next;
     u32 kid[2];
@@ -784,7 +790,7 @@
     return proc_encryption_packets( info, a );
 }
 
-static void
+static int
 proc_compressed( CTX c, PACKET *pkt )
 {
     PKT_compressed *zd = pkt->pkt.compressed;
@@ -803,6 +809,7 @@
 	log_error("uncompressing failed: %s\n", g10_errstr(rc));
     free_packet(pkt);
     c->last_was_session_key = 0;
+    return rc;
 }
 
 /****************
@@ -1233,14 +1240,37 @@
 }
 
 
-int
+static int
+check_nesting (CTX c)
+{
+  int level;
+
+  for (level = 0; c; c = c->anchor)
+    level++;
+
+  if (level > MAX_NESTING_DEPTH)
+    {
+      log_error ("input data with too deeply nested packets\n");
+      write_status_text (STATUS_UNEXPECTED, "1");
+      return G10ERR_UNEXPECTED;
+    }
+  return 0;
+}
+
+
+static int
 do_proc_packets( CTX c, IOBUF a )
 {
-    PACKET *pkt = xmalloc( sizeof *pkt );
-    int rc=0;
-    int any_data=0;
+    PACKET *pkt;
+    int rc = 0;
+    int any_data = 0;
     int newpkt;
 
+    rc = check_nesting (c);
+    if (rc)
+      return rc;
+
+    pkt = xmalloc( sizeof *pkt );
     c->iobuf = a;
     init_packet(pkt);
     while( (rc=parse_packet(a, pkt)) != -1 ) {
@@ -1260,7 +1290,7 @@
 	      case PKT_SYMKEY_ENC:  proc_symkey_enc( c, pkt ); break;
 	      case PKT_ENCRYPTED:
 	      case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
-	      case PKT_COMPRESSED:  proc_compressed( c, pkt ); break;
+	      case PKT_COMPRESSED:  rc = proc_compressed( c, pkt ); break;
 	      default: newpkt = 0; break;
 	    }
 	}
@@ -1278,7 +1308,7 @@
 		goto leave;
 	      case PKT_SIGNATURE:   newpkt = add_signature( c, pkt ); break;
 	      case PKT_PLAINTEXT:   proc_plaintext( c, pkt ); break;
-	      case PKT_COMPRESSED:  proc_compressed( c, pkt ); break;
+	      case PKT_COMPRESSED:  rc = proc_compressed( c, pkt ); break;
 	      case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
               case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
 	      default: newpkt = 0; break;
@@ -1298,7 +1328,7 @@
 	      case PKT_ENCRYPTED:
 	      case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
 	      case PKT_PLAINTEXT:   proc_plaintext( c, pkt ); break;
-	      case PKT_COMPRESSED:  proc_compressed( c, pkt ); break;
+	      case PKT_COMPRESSED:  rc = proc_compressed( c, pkt ); break;
 	      case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
 	      case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
 	      default: newpkt = 0; break;
@@ -1323,13 +1353,17 @@
 	      case PKT_ENCRYPTED:
 	      case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
 	      case PKT_PLAINTEXT:   proc_plaintext( c, pkt ); break;
-	      case PKT_COMPRESSED:  proc_compressed( c, pkt ); break;
+	      case PKT_COMPRESSED:  rc = proc_compressed( c, pkt ); break;
 	      case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
               case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
 	      case PKT_RING_TRUST:  newpkt = add_ring_trust( c, pkt ); break;
 	      default: newpkt = 0; break;
 	    }
 	}
+
+        if (rc)
+          goto leave;
+
         /* This is a very ugly construct and frankly, I don't remember why
          * I used it.  Adding the MDC check here is a hack.
          * The right solution is to initiate another context for encrypted
--- a/util/iobuf.c
+++ b/util/iobuf.c
@@ -27,7 +27,7 @@
 #include <assert.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h> 
+#include <fcntl.h>
 #include <unistd.h>
 #ifdef HAVE_DOSISH_SYSTEM
 #include <windows.h>
@@ -41,13 +41,13 @@
 #include "util.h"
 #include "dynload.h"
 #include "iobuf.h"
- 
+
 #ifdef __VMS
 # include "vms.h"
 # define open open_vms
 #endif /* def __VMS */
 
-/* The size of the internal buffers. 
+/* The size of the internal buffers.
    NOTE: If you change this value you MUST also adjust the regression
    test "armored_key_8192" and "nopad_armored_msg" in armor.test! */
 #define IOBUF_BUFFER_SIZE  8192
@@ -55,6 +55,11 @@
 
 #undef FILE_FILTER_USES_STDIO
 
+/* To avoid a potential DoS with compression packets we better limit
+   the number of filters in a chain.  */
+#define MAX_NESTING_FILTER 64
+
+
 #ifdef HAVE_DOSISH_SYSTEM
 #define USE_SETMODE 1
 #endif
@@ -76,8 +81,8 @@
  } file_filter_ctx_t ;
 #else
 #define my_fileno(a)  (a)
-#define my_fopen_ro(a,b) fd_cache_open ((a),(b)) 
-#define my_fopen(a,b) direct_open ((a),(b)) 
+#define my_fopen_ro(a,b) fd_cache_open ((a),(b))
+#define my_fopen(a,b) direct_open ((a),(b))
 #ifdef HAVE_DOSISH_SYSTEM
 typedef HANDLE FILEP_OR_FD;
 #define INVALID_FP  ((HANDLE)-1)
@@ -99,7 +104,7 @@
      char fname[1]; /* name of the file */
  } file_filter_ctx_t ;
 
- struct close_cache_s { 
+ struct close_cache_s {
     struct close_cache_s *next;
     FILEP_OR_FD fp;
     char fname[1];
@@ -153,7 +158,7 @@
 #ifdef HAVE_DOSISH_SYSTEM
   for (; *a && *b; a++, b++)
     {
-      if (*a != *b && !((*a == '/' && *b == '\\') 
+      if (*a != *b && !((*a == '/' && *b == '\\')
                         || (*a == '\\' && *b == '/')) )
         break;
     }
@@ -295,7 +300,7 @@
     {
         struct stat buf;
         int rc = stat( fname, &buf );
-        
+
         /* Don't allow iobufs on directories */
         if( !rc && S_ISDIR(buf.st_mode) && !S_ISREG(buf.st_mode) )
             return __set_errno( EISDIR );
@@ -308,7 +313,7 @@
 
 
 /*
- * Instead of closing an FD we keep it open and cache it for later reuse 
+ * Instead of closing an FD we keep it open and cache it for later reuse
  * Note that this caching strategy only works if the process does not chdir.
  */
 static void
@@ -471,8 +476,8 @@
     if( control == IOBUFCTRL_UNDERFLOW ) {
 	assert( size ); /* need a buffer */
 	if ( a->eof_seen) {
-	    rc = -1;		
-	    *ret_len = 0;	
+	    rc = -1;
+	    *ret_len = 0;
 	}
 	else {
 #ifdef HAVE_DOSISH_SYSTEM
@@ -606,8 +611,8 @@
     if( control == IOBUFCTRL_UNDERFLOW ) {
 	assert( size ); /* need a buffer */
 	if ( a->eof_seen) {
-	    rc = -1;		
-	    *ret_len = 0;	
+	    rc = -1;
+	    *ret_len = 0;
 	}
 	else {
             int nread;
@@ -1076,7 +1081,7 @@
         fname += 2;
         for (i=0; digitp (fname+i); i++ )
             ;
-        if ( !fname[i] ) 
+        if ( !fname[i] )
             return atoi (fname);
     }
     return -1;
@@ -1189,7 +1194,7 @@
     sock_filter( scx, IOBUFCTRL_INIT, NULL, NULL, &len );
     if( DBG_IOBUF )
 	log_debug("iobuf-%d.%d: sockopen `%s'\n", a->no, a->subno, scx->fname);
-    iobuf_ioctl (a,3,1,NULL); /* disable fd caching */ 
+    iobuf_ioctl (a,3,1,NULL); /* disable fd caching */
 #else
     a = iobuf_fdopen (fd, mode);
 #endif
@@ -1233,7 +1238,7 @@
     file_filter( fcx, IOBUFCTRL_DESC, NULL, (byte*)&a->desc, &len );
     file_filter( fcx, IOBUFCTRL_INIT, NULL, NULL, &len );
     if( DBG_IOBUF )
-	log_debug("iobuf-%d.%d: create `%s'\n", a->no, a->subno, 
+	log_debug("iobuf-%d.%d: create `%s'\n", a->no, a->subno,
                   a->desc?a->desc:"?" );
 
     return a;
@@ -1267,7 +1272,7 @@
     file_filter( fcx, IOBUFCTRL_DESC, NULL, (byte*)&a->desc, &len );
     file_filter( fcx, IOBUFCTRL_INIT, NULL, NULL, &len );
     if( DBG_IOBUF )
-	log_debug("iobuf-%d.%d: append `%s'\n", a->no, a->subno, 
+	log_debug("iobuf-%d.%d: append `%s'\n", a->no, a->subno,
                   a->desc?a->desc:"?" );
 
     return a;
@@ -1296,7 +1301,7 @@
     file_filter( fcx, IOBUFCTRL_DESC, NULL, (byte*)&a->desc, &len );
     file_filter( fcx, IOBUFCTRL_INIT, NULL, NULL, &len );
     if( DBG_IOBUF )
-	log_debug("iobuf-%d.%d: openrw `%s'\n", a->no, a->subno, 
+	log_debug("iobuf-%d.%d: openrw `%s'\n", a->no, a->subno,
                   a->desc?a->desc:"?");
 
     return a;
@@ -1309,7 +1314,7 @@
     if ( cmd == 1 ) {  /* keep system filepointer/descriptor open */
         if( DBG_IOBUF )
             log_debug("iobuf-%d.%d: ioctl `%s' keep=%d\n",
-                      a? a->no:-1, a?a->subno:-1, 
+                      a? a->no:-1, a?a->subno:-1,
                       a&&a->desc?a->desc:"?", intval );
         for( ; a; a = a->chain )
             if( !a->chain && a->filter == file_filter ) {
@@ -1339,7 +1344,7 @@
     else if ( cmd == 3 ) {  /* disallow/allow caching */
         if( DBG_IOBUF )
             log_debug("iobuf-%d.%d: ioctl `%s' no_cache=%d\n",
-                      a? a->no:-1, a?a->subno:-1, 
+                      a? a->no:-1, a?a->subno:-1,
                       a&&a->desc?a->desc:"?", intval );
         for( ; a; a = a->chain )
             if( !a->chain && a->filter == file_filter ) {
@@ -1403,6 +1408,12 @@
 
     if( a->use == 2 && (rc=iobuf_flush(a)) )
 	return rc;
+
+    if (a->subno >= MAX_NESTING_FILTER) {
+        log_error ("i/o filter too deeply nested - corrupted data?\n");
+        return G10ERR_UNEXPECTED;
+    }
+
     /* make a copy of the current stream, so that
      * A is the new stream and B the original one.
      * The contents of the buffers are transferred to the
@@ -1449,7 +1460,7 @@
     f( ov, IOBUFCTRL_DESC, NULL, (byte*)&a->desc, &dummy_len );
 
     if( DBG_IOBUF ) {
-	log_debug("iobuf-%d.%d: push `%s'\n", a->no, a->subno, 
+	log_debug("iobuf-%d.%d: push `%s'\n", a->no, a->subno,
                   a->desc?a->desc:"?" );
 	print_chain( a );
     }
@@ -1921,7 +1932,7 @@
     if (overflow)
       *overflow = 0;
 
-    if (a->directfp) 
+    if (a->directfp)
       {
 	FILE *fp = a->directfp;
 
@@ -1949,14 +1960,14 @@
 
 #if defined(HAVE_DOSISH_SYSTEM) && !defined(FILE_FILTER_USES_STDIO)
             ulong size;
-            static int (* __stdcall get_file_size_ex) 
+            static int (* __stdcall get_file_size_ex)
               (void *handle, LARGE_INTEGER *size);
             static int get_file_size_ex_initialized;
 
             if (!get_file_size_ex_initialized)
               {
                 void *handle;
-                
+
                 handle = dlopen ("kernel32.dll", RTLD_LAZY);
                 if (handle)
                   {
@@ -1974,14 +1985,14 @@
                    return a proper error in case a file is larger than
                    4GB. */
                 LARGE_INTEGER size;
-                
+
                 if (get_file_size_ex (fp, &size))
                   {
                     if (!size.u.HighPart)
                       return size.u.LowPart;
                     if (overflow)
                       *overflow = 1;
-                    return 0; 
+                    return 0;
                   }
               }
             else
@@ -2007,7 +2018,7 @@
 
 /* Return the file descriptor of the underlying file or -1 if it is
    not available.  */
-int 
+int
 iobuf_get_fd (IOBUF a)
 {
   if (a->directfp)
@@ -2260,7 +2271,7 @@
 #ifdef _WIN32
     {
         int x;
-            
+
         if  ( fd <= 2 )
             return fd; /* do not do this for error, stdin, stdout, stderr */
 
@@ -2281,17 +2292,17 @@
 translate_file_handle ( int fd, int for_write )
 {
 #ifdef _WIN32
-#ifdef FILE_FILTER_USES_STDIO  
+#ifdef FILE_FILTER_USES_STDIO
     fd = iobuf_translate_file_handle (fd, for_write);
 #else
     {
         int x;
 
-        if  ( fd == 0 ) 
+        if  ( fd == 0 )
             x = (int)GetStdHandle (STD_INPUT_HANDLE);
-        else if (fd == 1)    
+        else if (fd == 1)
             x = (int)GetStdHandle (STD_OUTPUT_HANDLE);
-        else if (fd == 2)    
+        else if (fd == 2)
             x = (int)GetStdHandle (STD_ERROR_HANDLE);
         else
             x = fd;
