[Pkg-gnupg-commit] [gnupg2] 95/124: gpg: Fix actual leak and possible leaks in the packet parser.

Daniel Kahn Gillmor dkg at fifthhorseman.net
Wed Apr 5 15:55:36 UTC 2017


This is an automated email from the git hooks/post-receive script.

dkg pushed a commit to branch experimental
in repository gnupg2.

commit 7bf24e8146116a30c4c9d7b6dbf8bbb27fc35971
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Mar 30 16:01:52 2017 +0200

    gpg: Fix actual leak and possible leaks in the packet parser.
    
    * g10/packet.h (struct parse_packet_ctx_s): Change LAST_PKT deom a
    pointer to its struct.
    (init_parse_packet): Adjust for LAST_PKT not being a pointer.
    * g10/parse-packet.c (parse): Ditto. Free the last packet before
    storing a new one in case of a deep link.
    (parse_ring_trust): Adjust for LAST_PKT not being a pointer.
    * g10/free-packet.c (free_packet): Ditto.
    * g10/t-keydb-get-keyblock.c (do_test): Release keyblock.
    --
    
    Fixes-commit: afa86809087909a8ba2f9356588bf90cc923529c
    Signed-off-by: Werner Koch <wk at gnupg.org>
---
 g10/build-packet.c         |  2 +-
 g10/free-packet.c          | 12 ++++++++----
 g10/packet.h               |  5 +++--
 g10/parse-packet.c         | 25 +++++++++++++------------
 g10/t-keydb-get-keyblock.c |  1 +
 5 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/g10/build-packet.c b/g10/build-packet.c
index 1ee57e0..fa2674b 100644
--- a/g10/build-packet.c
+++ b/g10/build-packet.c
@@ -420,7 +420,7 @@ do_user_id( IOBUF out, int ctb, PKT_user_id *uid )
   log_assert (ctb_pkttype (ctb) == PKT_USER_ID
               || ctb_pkttype (ctb) == PKT_ATTRIBUTE);
 
-  /* We need to take special care that doe user ID with a length of 0:
+  /* We need to take special care of a user ID with a length of 0:
    * Without forcing HDRLEN to 2 in this case an indeterminate length
    * packet would be written which is not allowed.  Note that we are
    * always called with a CTB indicating an old packet header format,
diff --git a/g10/free-packet.c b/g10/free-packet.c
index c144246..cd222a2 100644
--- a/g10/free-packet.c
+++ b/g10/free-packet.c
@@ -409,14 +409,15 @@ free_packet (PACKET *pkt, parse_packet_ctx_t parsectx)
 {
   if (!pkt || !pkt->pkt.generic)
     {
-      if (parsectx && parsectx->last_pkt)
+      if (parsectx && parsectx->last_pkt.pkt.generic)
         {
           if (parsectx->free_last_pkt)
             {
-              free_packet (parsectx->last_pkt, NULL);
+              free_packet (&parsectx->last_pkt, NULL);
               parsectx->free_last_pkt = 0;
             }
-          parsectx->last_pkt = NULL;
+          parsectx->last_pkt.pkttype = 0;
+          parsectx->last_pkt.pkt.generic = NULL;
         }
       return;
     }
@@ -427,8 +428,11 @@ free_packet (PACKET *pkt, parse_packet_ctx_t parsectx)
   /* If we have a parser context holding PKT then do not free the
    * packet but set a flag that the packet in the parser context is
    * now a deep copy.  */
-  if (parsectx && parsectx->last_pkt == pkt && !parsectx->free_last_pkt)
+  if (parsectx && !parsectx->free_last_pkt
+      && parsectx->last_pkt.pkttype == pkt->pkttype
+      && parsectx->last_pkt.pkt.generic == pkt->pkt.generic)
     {
+      parsectx->last_pkt = *pkt;
       parsectx->free_last_pkt = 1;
       pkt->pkt.generic = NULL;
       return;
diff --git a/g10/packet.h b/g10/packet.h
index b23298a..f5f22b6 100644
--- a/g10/packet.h
+++ b/g10/packet.h
@@ -621,7 +621,7 @@ int set_packet_list_mode( int mode );
 struct parse_packet_ctx_s
 {
   iobuf_t inp;       /* The input stream with the packets.  */
-  PACKET *last_pkt;  /* The last parsed packet.  */
+  struct packet_struct last_pkt; /* The last parsed packet.  */
   int free_last_pkt; /* Indicates that LAST_PKT must be freed.  */
   int skip_meta;     /* Skip right trust packets.  */
 };
@@ -629,7 +629,8 @@ typedef struct parse_packet_ctx_s *parse_packet_ctx_t;
 
 #define init_parse_packet(a,i) do { \
     (a)->inp = (i);                 \
-    (a)->last_pkt = NULL;           \
+    (a)->last_pkt.pkttype = 0;      \
+    (a)->last_pkt.pkt.generic= NULL;\
     (a)->free_last_pkt = 0;         \
     (a)->skip_meta = 0;             \
   } while (0)
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index df04fbc..793e198 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -833,14 +833,15 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
     }
 
   /* Store a shallow copy of certain packets in the context.  */
+  free_packet (NULL, ctx);
   if (!rc && (pkttype == PKT_PUBLIC_KEY
               || pkttype == PKT_SECRET_KEY
               || pkttype == PKT_USER_ID
               || pkttype == PKT_ATTRIBUTE
               || pkttype == PKT_SIGNATURE))
-    ctx->last_pkt = pkt;
-  else
-    ctx->last_pkt = NULL;
+    {
+      ctx->last_pkt = *pkt;
+    }
 
  leave:
   /* FIXME: We leak in case of an error (see the xmalloc's above).  */
@@ -2992,12 +2993,12 @@ parse_ring_trust (parse_packet_ctx_t ctx, unsigned long pktlen)
 
   /* Now transfer the data to the respective packet.  Do not do this
    * if SKIP_META is set.  */
-  if (!ctx->last_pkt || ctx->skip_meta)
+  if (!ctx->last_pkt.pkt.generic || ctx->skip_meta)
     ;
   else if (rt.subtype == RING_TRUST_SIG
-           && ctx->last_pkt->pkttype == PKT_SIGNATURE)
+           && ctx->last_pkt.pkttype == PKT_SIGNATURE)
     {
-      PKT_signature *sig = ctx->last_pkt->pkt.signature;
+      PKT_signature *sig = ctx->last_pkt.pkt.signature;
 
       if ((rt.sigcache & 1))
         {
@@ -3006,10 +3007,10 @@ parse_ring_trust (parse_packet_ctx_t ctx, unsigned long pktlen)
         }
     }
   else if (rt.subtype == RING_TRUST_UID
-           && (ctx->last_pkt->pkttype == PKT_USER_ID
-               || ctx->last_pkt->pkttype == PKT_ATTRIBUTE))
+           && (ctx->last_pkt.pkttype == PKT_USER_ID
+               || ctx->last_pkt.pkttype == PKT_ATTRIBUTE))
     {
-      PKT_user_id *uid = ctx->last_pkt->pkt.user_id;
+      PKT_user_id *uid = ctx->last_pkt.pkt.user_id;
 
       uid->keysrc = rt.keysrc;
       uid->keyupdate = rt.keyupdate;
@@ -3017,10 +3018,10 @@ parse_ring_trust (parse_packet_ctx_t ctx, unsigned long pktlen)
       rt.url = NULL;
     }
   else if (rt.subtype == RING_TRUST_KEY
-           && (ctx->last_pkt->pkttype == PKT_PUBLIC_KEY
-               || ctx->last_pkt->pkttype == PKT_SECRET_KEY))
+           && (ctx->last_pkt.pkttype == PKT_PUBLIC_KEY
+               || ctx->last_pkt.pkttype == PKT_SECRET_KEY))
     {
-      PKT_public_key *pk = ctx->last_pkt->pkt.public_key;
+      PKT_public_key *pk = ctx->last_pkt.pkt.public_key;
 
       pk->keysrc = rt.keysrc;
       pk->keyupdate = rt.keyupdate;
diff --git a/g10/t-keydb-get-keyblock.c b/g10/t-keydb-get-keyblock.c
index 993d879..167a9bb 100644
--- a/g10/t-keydb-get-keyblock.c
+++ b/g10/t-keydb-get-keyblock.c
@@ -61,4 +61,5 @@ do_test (int argc, char *argv[])
   TEST_P ("", ! rc);
 
   keydb_release (hd1);
+  release_kbnode (kb1);
 }

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gnupg2.git



More information about the Pkg-gnupg-commit mailing list