[git] GnuPG - branch, master, updated. gnupg-2.1.7-42-g09f2a7b

by Neal H. Walfield cvs at cvs.gnupg.org
Fri Aug 21 14:21:23 CEST 2015


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".

The branch, master has been updated
       via  09f2a7bca624d0492e1d7ab29ce19542249c13ff (commit)
       via  4f37820334fadd8c5036ea6c42f3dc242665c4a9 (commit)
       via  b3226cadf9bbef4a367072396e5b0abf37afff2d (commit)
       via  0143d5c1ca4d12ac252c14f01931f48131591065 (commit)
       via  48e792cc951a9d00fad0691ef7411c9e22cf675a (commit)
       via  73af66a0aada8f30d8f400fdc4f69e233fb53089 (commit)
      from  b8adfc4186fa209dce3e7ca763cec8576b8ee291 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 09f2a7bca624d0492e1d7ab29ce19542249c13ff
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 11:55:15 2015 +0200

    common: Don't incorrectly reject 4 GB - 1 sized packets.
    
    * g10/parse-packet.c (parse): Don't reject 4 GB - 1 sized packets.
    Add the constraint that the type must be 63.
    * kbx/keybox-openpgp.c (next_packet): Likewise.
    * tests/openpgp/4gb-packet.asc: New file.
    * tests/openpgp/4gb-packet.test: New file.
    * tests/openpgp/Makefile.am (TESTS): Add 4gb-packet.test.
    (TEST_FILES): Add 4gb-packet.asc.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index bc99653..edebbe7 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -643,7 +643,14 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
 	}
     }
 
-  if (pktlen == (unsigned long) (-1))
+  /* Sometimes the decompressing layer enters an error state in which
+     it simply outputs 0xff for every byte read.  If we have a stream
+     of 0xff bytes, then it will be detected as a new format packet
+     with type 63 and a 4-byte encoded length that is 4G-1.  Since
+     packets with type 63 are private and we use them as a control
+     packet, which won't be 4 GB, we reject such packets as
+     invalid.  */
+  if (pkttype == 63 && pktlen == 0xFFFFFFFF)
     {
       /* With some probability this is caused by a problem in the
        * the uncompressing layer - in some error cases it just loops
diff --git a/kbx/keybox-openpgp.c b/kbx/keybox-openpgp.c
index 2cac242..a5f602b 100644
--- a/kbx/keybox-openpgp.c
+++ b/kbx/keybox-openpgp.c
@@ -139,7 +139,14 @@ next_packet (unsigned char const **bufptr, size_t *buflen,
       return gpg_error (GPG_ERR_UNEXPECTED);
     }
 
-  if (pktlen == (unsigned long)(-1))
+  if (pkttype == 63 && pktlen == 0xFFFFFFFF)
+    /* Sometimes the decompressing layer enters an error state in
+       which it simply outputs 0xff for every byte read.  If we have a
+       stream of 0xff bytes, then it will be detected as a new format
+       packet with type 63 and a 4-byte encoded length that is 4G-1.
+       Since packets with type 63 are private and we use them as a
+       control packet, which won't be 4 GB, we reject such packets as
+       invalid.  */
     return gpg_error (GPG_ERR_INV_PACKET);
 
   if (pktlen > len)
diff --git a/tests/openpgp/4gb-packet.asc b/tests/openpgp/4gb-packet.asc
new file mode 100644
index 0000000..7e5d6f3
Binary files /dev/null and b/tests/openpgp/4gb-packet.asc differ
diff --git a/tests/openpgp/4gb-packet.test b/tests/openpgp/4gb-packet.test
new file mode 100755
index 0000000..f8e43c8
--- /dev/null
+++ b/tests/openpgp/4gb-packet.test
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+. $srcdir/defs.inc || exit 3
+
+# GnuPG through 2.1.7 would incorrect mark packets whose size is
+# 2^32-1 as invalid and exit with status code 2.
+i=$srcdir/4gb-packet.asc
+
+if ! $GPG --list-packets $i
+then
+  echo Failed to parse 4GB packet.
+  exit 1
+else
+  echo Can parse 4GB packets.
+  exit 0
+fi
diff --git a/tests/openpgp/Makefile.am b/tests/openpgp/Makefile.am
index dae8c11..4fdb0a6 100644
--- a/tests/openpgp/Makefile.am
+++ b/tests/openpgp/Makefile.am
@@ -38,7 +38,7 @@ TESTS = version.test mds.test \
 	armdetachm.test detachm.test genkey1024.test \
 	conventional.test conventional-mdc.test \
 	multisig.test verify.test armor.test \
-	import.test ecc.test finish.test
+	import.test ecc.test 4gb-packet.test finish.test
 
 
 TEST_FILES = pubring.asc secring.asc plain-1o.asc plain-2o.asc plain-3o.asc \
@@ -46,7 +46,7 @@ TEST_FILES = pubring.asc secring.asc plain-1o.asc plain-2o.asc plain-3o.asc \
 	     pubring.pkr.asc secring.skr.asc secdemo.asc pubdemo.asc \
              gpg.conf.tmpl gpg-agent.conf.tmpl \
 	     bug537-test.data.asc bug894-test.asc \
-	     bug1223-good.asc bug1223-bogus.asc
+	     bug1223-good.asc bug1223-bogus.asc 4gb-packet.asc
 
 data_files = data-500 data-9000 data-32000 data-80000 plain-large
 

commit 4f37820334fadd8c5036ea6c42f3dc242665c4a9
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 10:38:41 2015 +0200

    common: Don't assume on-disk layout matches in-memory layout.
    
    * g10/packet.h (PKT_signature): Change revkey's type from a struct
    revocation_key ** to a struct revocation_key *.  Update users.
    
    --
    revkey was a pointer into the raw data.  But, C doesn't guarantee that
    there is no padding.  Thus, we copy the data.
    
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/export.c b/g10/export.c
index 5050128..62802d3 100644
--- a/g10/export.c
+++ b/g10/export.c
@@ -1011,7 +1011,7 @@ do_export_stream (ctrl_t ctrl, iobuf_t out, strlist_t users, int secret,
                   int i;
 
                   for (i=0;i<node->pkt->pkt.signature->numrevkeys;i++)
-                    if ( (node->pkt->pkt.signature->revkey[i]->class & 0x40))
+                    if ( (node->pkt->pkt.signature->revkey[i].class & 0x40))
                       break;
 
                   if (i < node->pkt->pkt.signature->numrevkeys)
diff --git a/g10/getkey.c b/g10/getkey.c
index 3a60161..6e85834 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -720,7 +720,7 @@ get_pubkey_byname (ctrl_t ctrl, GETKEY_CTX * retctx, PKT_public_key * pk,
 
      ANYLOCALFIRST is set if the search order has the local method
      before any other or if "local" is used first by default.  This
-     makes sure that if a RETCTX is used it gets only set if a local
+     makes sure that if a RETCTX is used it is only set if a local
      search has precedence over the other search methods and only then
      a followup call to get_pubkey_next shall succeed.  */
   if (!no_akl)
@@ -1606,7 +1606,7 @@ merge_selfsigs_main (KBNODE keyblock, int *r_revoked,
 
 		      for (i = 0; i < sig->numrevkeys; i++)
 			memcpy (&pk->revkey[pk->numrevkeys++],
-				sig->revkey[i],
+				&sig->revkey[i],
 				sizeof (struct revocation_key));
 		    }
 
diff --git a/g10/import.c b/g10/import.c
index e92769d..60a037b 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -2397,7 +2397,7 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
 	    {
 	      u32 keyid[2];
 
-	      keyid_from_fingerprint(sig->revkey[idx]->fpr,
+	      keyid_from_fingerprint(sig->revkey[idx].fpr,
 				     MAX_FINGERPRINT_LEN,keyid);
 
 	      for(inode=keyblock->next;inode;inode=inode->next)
@@ -2416,7 +2416,7 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
                          itself? */
 		      int rc;
 
-		      rc=get_pubkey_byfprint_fast (NULL,sig->revkey[idx]->fpr,
+		      rc=get_pubkey_byfprint_fast (NULL,sig->revkey[idx].fpr,
                                                    MAX_FINGERPRINT_LEN);
 		      if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY
                           || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY)
@@ -2432,13 +2432,13 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
 					 " fetching revocation key %s\n"),
 				       tempkeystr,keystr(keyid));
 			      keyserver_import_fprint (ctrl,
-                                                       sig->revkey[idx]->fpr,
+                                                       sig->revkey[idx].fpr,
                                                        MAX_FINGERPRINT_LEN,
                                                        opt.keyserver);
 
 			      /* Do we have it now? */
 			      rc=get_pubkey_byfprint_fast (NULL,
-						     sig->revkey[idx]->fpr,
+						     sig->revkey[idx].fpr,
 						     MAX_FINGERPRINT_LEN);
 			    }
 
diff --git a/g10/packet.h b/g10/packet.h
index 8bd5fc4..826963e 100644
--- a/g10/packet.h
+++ b/g10/packet.h
@@ -167,7 +167,7 @@ typedef struct
   byte    trust_depth;
   byte    trust_value;
   const byte *trust_regexp;
-  struct revocation_key **revkey;
+  struct revocation_key *revkey;
   int numrevkeys;
   pka_info_t *pka_info;      /* Malloced PKA data or NULL if not
                                 available.  See also flags.pka_tried. */
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index 1467dc3..bc99653 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -1711,25 +1711,31 @@ parse_sig_subpkt2 (PKT_signature * sig, sigsubpkttype_t reqtype)
 void
 parse_revkeys (PKT_signature * sig)
 {
-  struct revocation_key *revkey;
+  const byte *revkey;
   int seq = 0;
   size_t len;
 
   if (sig->sig_class != 0x1F)
     return;
 
-  while ((revkey =
-	  (struct revocation_key *) enum_sig_subpkt (sig->hashed,
-						     SIGSUBPKT_REV_KEY,
-						     &len, &seq, NULL)))
+  while ((revkey = enum_sig_subpkt (sig->hashed, SIGSUBPKT_REV_KEY,
+				    &len, &seq, NULL)))
     {
-      if (len == sizeof (struct revocation_key)
-          && (revkey->class & 0x80))  /* 0x80 bit must be set.  */
+      if (/* The only valid length is 22 bytes.  See RFC 4880
+	     5.2.3.15.  */
+	  len == 22
+	  /* 0x80 bit must be set on the class.  */
+          && (revkey[0] & 0x80))
 	{
 	  sig->revkey = xrealloc (sig->revkey,
-				  sizeof (struct revocation_key *) *
+				  sizeof (struct revocation_key) *
 				  (sig->numrevkeys + 1));
-	  sig->revkey[sig->numrevkeys] = revkey;
+
+	  /* Copy the individual fields.  */
+	  sig->revkey[sig->numrevkeys].class = revkey[0];
+	  sig->revkey[sig->numrevkeys].algid = revkey[1];
+	  memcpy (sig->revkey[sig->numrevkeys].fpr, &revkey[2], 20);
+
 	  sig->numrevkeys++;
 	}
     }
diff --git a/g10/revoke.c b/g10/revoke.c
index 6e82187..eb3a989 100644
--- a/g10/revoke.c
+++ b/g10/revoke.c
@@ -383,11 +383,11 @@ gen_desig_revoke( const char *uname, strlist_t locusr )
 		    for(j=0;j<signode->pkt->pkt.signature->numrevkeys;j++)
 		      {
 			if(pk->revkey[i].class==
-			   signode->pkt->pkt.signature->revkey[j]->class &&
+			   signode->pkt->pkt.signature->revkey[j].class &&
 			   pk->revkey[i].algid==
-			   signode->pkt->pkt.signature->revkey[j]->algid &&
+			   signode->pkt->pkt.signature->revkey[j].algid &&
 			   memcmp(pk->revkey[i].fpr,
-				  signode->pkt->pkt.signature->revkey[j]->fpr,
+				  signode->pkt->pkt.signature->revkey[j].fpr,
 				  MAX_FINGERPRINT_LEN)==0)
 			  {
 			    revkey=signode->pkt->pkt.signature;

commit b3226cadf9bbef4a367072396e5b0abf37afff2d
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 09:47:57 2015 +0200

    common: Don't incorrectly copy packets with partial lengths.
    
    * g10/parse-packet.c (parse): We don't handle copying packets with a
    partial body length to an output stream.  If this occurs, log an error
    and abort.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index 6f44efa..1467dc3 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -654,6 +654,17 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
 
   if (out && pkttype)
     {
+      /* This type of copying won't work if the packet uses a partial
+	 body length.  (In other words, this only works if HDR is
+	 actually the length.)  Currently, no callers require this
+	 functionality so we just log this as an error.  */
+      if (partial)
+	{
+	  log_error ("parse: Can't copy partial packet.  Aborting.\n");
+	  rc = gpg_error (GPG_ERR_INV_PACKET);
+	  goto leave;
+	}
+
       rc = iobuf_write (out, hdr, hdrlen);
       if (!rc)
 	rc = copy_packet (inp, out, pkttype, pktlen, partial);

commit 0143d5c1ca4d12ac252c14f01931f48131591065
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 09:35:09 2015 +0200

    common: Check parameters more rigorously.
    
    * g10/parse-packet.c (dbg_copy_all_packets): Check that OUT is not
    NULL.
    (copy_all_packets): Likewise.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index 4ba9419..6f44efa 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -336,6 +336,10 @@ dbg_copy_all_packets (IOBUF inp, IOBUF out, const char *dbg_f, int dbg_l)
 {
   PACKET pkt;
   int skip, rc = 0;
+
+  if (! out)
+    log_bug ("copy_all_packets: OUT may not be NULL.\n");
+
   do
     {
       init_packet (&pkt);
@@ -351,6 +355,10 @@ copy_all_packets (IOBUF inp, IOBUF out)
 {
   PACKET pkt;
   int skip, rc = 0;
+
+  if (! out)
+    log_bug ("copy_all_packets: OUT may not be NULL.\n");
+
   do
     {
       init_packet (&pkt);

commit 48e792cc951a9d00fad0691ef7411c9e22cf675a
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 09:32:58 2015 +0200

    common: Don't continuing processing on error.
    
    * g10/parse-packet.c (dbg_parse_packet): Also return if parse returns
    an error.
    (parse_packet): Likewise.
    (dbg_search_packet): Likewise.
    (search_packet): Likewise.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index 2afba54..4ba9419 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -272,7 +272,7 @@ dbg_parse_packet (IOBUF inp, PACKET *pkt, const char *dbg_f, int dbg_l)
     {
       rc = parse (inp, pkt, 0, NULL, &skip, NULL, 0, "parse", dbg_f, dbg_l);
     }
-  while (skip);
+  while (skip && ! rc);
   return rc;
 }
 #else /*!DEBUG_PARSE_PACKET*/
@@ -285,7 +285,7 @@ parse_packet (IOBUF inp, PACKET * pkt)
     {
       rc = parse (inp, pkt, 0, NULL, &skip, NULL, 0);
     }
-  while (skip);
+  while (skip && ! rc);
   return rc;
 }
 #endif /*!DEBUG_PARSE_PACKET*/
@@ -308,7 +308,7 @@ dbg_search_packet (IOBUF inp, PACKET * pkt, off_t * retpos, int with_uid,
 	parse (inp, pkt, with_uid ? 2 : 1, retpos, &skip, NULL, 0, "search",
 	       dbg_f, dbg_l);
     }
-  while (skip);
+  while (skip && ! rc);
   return rc;
 }
 #else /*!DEBUG_PARSE_PACKET*/
@@ -321,7 +321,7 @@ search_packet (IOBUF inp, PACKET * pkt, off_t * retpos, int with_uid)
     {
       rc = parse (inp, pkt, with_uid ? 2 : 1, retpos, &skip, NULL, 0);
     }
-  while (skip);
+  while (skip && ! rc);
   return rc;
 }
 #endif /*!DEBUG_PARSE_PACKET*/

commit 73af66a0aada8f30d8f400fdc4f69e233fb53089
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Aug 21 09:28:49 2015 +0200

    common: Better respect the packet's length when reading it.
    
    * g10/parse-packet.c (parse_signature): Make sure PKTLEN doesn't
    underflow.  Be more careful that a read doesn't read more data than
    PKTLEN says is available.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index cd202da..2afba54 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -1750,13 +1750,19 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
 
   if (!is_v4)
     {
+      if (pktlen == 0)
+	goto underflow;
       md5_len = iobuf_get_noeof (inp);
       pktlen--;
     }
+  if (pktlen == 0)
+    goto underflow;
   sig->sig_class = iobuf_get_noeof (inp);
   pktlen--;
   if (!is_v4)
     {
+      if (pktlen < 12)
+	goto underflow;
       sig->timestamp = read_32 (inp);
       pktlen -= 4;
       sig->keyid[0] = read_32 (inp);
@@ -1764,6 +1770,8 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
       sig->keyid[1] = read_32 (inp);
       pktlen -= 4;
     }
+  if (pktlen < 2)
+    goto underflow;
   sig->pubkey_algo = iobuf_get_noeof (inp);
   pktlen--;
   sig->digest_algo = iobuf_get_noeof (inp);
@@ -1772,8 +1780,12 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
   sig->flags.revocable = 1;
   if (is_v4) /* Read subpackets.  */
     {
+      if (pktlen < 2)
+	goto underflow;
       n = read_16 (inp);
       pktlen -= 2;  /* Length of hashed data. */
+      if (pktlen < n)
+	goto underflow;
       if (n > 10000)
 	{
 	  log_error ("signature packet: hashed data too long\n");
@@ -1798,8 +1810,12 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
 	    }
 	  pktlen -= n;
 	}
+      if (pktlen < 2)
+	goto underflow;
       n = read_16 (inp);
       pktlen -= 2;  /* Length of unhashed data.  */
+      if (pktlen < n)
+	goto underflow;
       if (n > 10000)
 	{
 	  log_error ("signature packet: unhashed data too long\n");
@@ -1826,15 +1842,8 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
 	}
     }
 
-  if (pktlen < 5)  /* Sanity check.  */
-    {
-      log_error ("packet(%d) too short\n", pkttype);
-      if (list_mode)
-        es_fputs (":signature packet: [too short]\n", listfp);
-      rc = GPG_ERR_INV_PACKET;
-      goto leave;
-    }
-
+  if (pktlen < 2)
+    goto underflow;
   sig->digest_start[0] = iobuf_get_noeof (inp);
   pktlen--;
   sig->digest_start[1] = iobuf_get_noeof (inp);
@@ -1981,6 +1990,15 @@ parse_signature (IOBUF inp, int pkttype, unsigned long pktlen,
  leave:
   iobuf_skip_rest (inp, pktlen, 0);
   return rc;
+
+ underflow:
+  log_error ("packet(%d) too short\n", pkttype);
+  if (list_mode)
+    es_fputs (":signature packet: [too short]\n", listfp);
+
+  iobuf_skip_rest (inp, pktlen, 0);
+
+  return GPG_ERR_INV_PACKET;
 }
 
 

-----------------------------------------------------------------------

Summary of changes:
 g10/export.c                  |   2 +-
 g10/getkey.c                  |   4 +-
 g10/import.c                  |   8 ++--
 g10/packet.h                  |   2 +-
 g10/parse-packet.c            |  96 ++++++++++++++++++++++++++++++++----------
 g10/revoke.c                  |   6 +--
 kbx/keybox-openpgp.c          |   9 +++-
 tests/openpgp/4gb-packet.asc  | Bin 0 -> 4983 bytes
 tests/openpgp/4gb-packet.test |  16 +++++++
 tests/openpgp/Makefile.am     |   4 +-
 10 files changed, 110 insertions(+), 37 deletions(-)
 create mode 100644 tests/openpgp/4gb-packet.asc
 create mode 100755 tests/openpgp/4gb-packet.test


hooks/post-receive
-- 
The GNU Privacy Guard
http://git.gnupg.org




More information about the Gnupg-commits mailing list