From 33a2362e566c0e0d7011abf2e5fa5704d7cb4206 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Mon, 12 Apr 2021 19:19:59 +0200
Subject: [PATCH] agent: Fix memory leaks

* agent/call-daemon.c (daemon_start): free wctp
* agent/call-scd.c (agent_card_pksign): return error instead of noop
(card_keyinfo_cb): free keyinfo.  Restructure to avoid a goto backwards.
* agent/protect.c (agent_get_shadow_info_type): allocate only as a last
action.  Catch xtrymalloc failure.
(agent_is_tpm2_key): Free buf.

--

Signed-off-by: Jakub Jelen <jjelen@redhat.com>

Additional changes are:
- Restructure to avoid a goto backwards.
- Catch xtrymalloc failure.

GnuPG-bug-id: 5393
Signed-off-by: Werner Koch <wk@gnupg.org>
---
 agent/call-daemon.c |  2 ++
 agent/call-scd.c    | 30 ++++++++++++++++--------------
 agent/protect.c     | 20 ++++++++++++--------
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/agent/call-daemon.c b/agent/call-daemon.c
index 144400875..3bf6bb793 100644
--- a/agent/call-daemon.c
+++ b/agent/call-daemon.c
@@ -512,6 +512,8 @@ daemon_start (enum daemon_type type, ctrl_t ctrl)
           log_error ("error spawning wait_child_thread: %s\n", strerror (err));
         npth_attr_destroy (&tattr);
       }
+    else
+      xfree (wctp);
   }
 
  leave:
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 3ede33c1d..395c13b34 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -487,7 +487,7 @@ agent_card_pksign (ctrl_t ctrl,
   /* FIXME: In the mdalgo case (INDATA,INDATALEN) might be long and
    * thus we can't convey it on a single Assuan line.  */
   if (!mdalgo)
-    gpg_error (GPG_ERR_NOT_IMPLEMENTED);
+    return gpg_error (GPG_ERR_NOT_IMPLEMENTED);
 
   if (indatalen*2 + 50 > DIM(line))
     return unlock_scd (ctrl, gpg_error (GPG_ERR_GENERAL));
@@ -921,6 +921,7 @@ card_keyinfo_cb (void *opaque, const char *line)
   struct card_keyinfo_parm_s *parm = opaque;
   const char *keyword = line;
   int keywordlen;
+  struct card_key_info_s *keyinfo = NULL;
 
   for (keywordlen=0; *line && !spacep (line); line++, keywordlen++)
     ;
@@ -931,7 +932,6 @@ card_keyinfo_cb (void *opaque, const char *line)
     {
       const char *s;
       int n;
-      struct card_key_info_s *keyinfo;
       struct card_key_info_s **l_p = &parm->list;
 
       while ((*l_p))
@@ -939,23 +939,13 @@ card_keyinfo_cb (void *opaque, const char *line)
 
       keyinfo = xtrycalloc (1, sizeof *keyinfo);
       if (!keyinfo)
-        {
-        alloc_error:
-          if (!parm->error)
-            parm->error = gpg_error_from_syserror ();
-          return 0;
-        }
+        goto alloc_error;
 
       for (n=0,s=line; hexdigitp (s); s++, n++)
         ;
 
       if (n != 40)
-        {
-        parm_error:
-          if (!parm->error)
-            parm->error = gpg_error (GPG_ERR_ASS_PARAMETER);
-          return 0;
-        }
+        goto parm_error;
 
       memcpy (keyinfo->keygrip, line, 40);
       keyinfo->keygrip[40] = 0;
@@ -1011,6 +1001,18 @@ card_keyinfo_cb (void *opaque, const char *line)
     err = handle_pincache_put (line);
 
   return err;
+
+ alloc_error:
+  xfree (keyinfo);
+  if (!parm->error)
+    parm->error = gpg_error_from_syserror ();
+  return 0;
+
+ parm_error:
+  xfree (keyinfo);
+  if (!parm->error)
+    parm->error = gpg_error (GPG_ERR_ASS_PARAMETER);
+  return 0;
 }
 
 
diff --git a/agent/protect.c b/agent/protect.c
index 76ead444b..2c63a85fe 100644
--- a/agent/protect.c
+++ b/agent/protect.c
@@ -1660,13 +1660,6 @@ agent_get_shadow_info_type (const unsigned char *shadowkey,
   n = snext (&s);
   if (!n)
     return gpg_error (GPG_ERR_INV_SEXP);
-  if (shadow_type) {
-    char *buf = xtrymalloc(n+1);
-    memcpy(buf, s, n);
-    buf[n] = '\0';
-    *shadow_type = buf;
-  }
-
   if (smatch (&s, n, "t1-v1") || smatch(&s, n, "tpm2-v1"))
     {
       if (*s != '(')
@@ -1676,6 +1669,17 @@ agent_get_shadow_info_type (const unsigned char *shadowkey,
     }
   else
     return gpg_error (GPG_ERR_UNSUPPORTED_PROTOCOL);
+
+  if (shadow_type)
+    {
+      char *buf = xtrymalloc(n+1);
+      if (!buf)
+        return gpg_error_from_syserror ();
+      memcpy (buf, s, n);
+      buf[n] = '\0';
+      *shadow_type = buf;
+    }
+
   return 0;
 }
 
@@ -1701,9 +1705,9 @@ agent_is_tpm2_key (gcry_sexp_t s_skey)
     return 0;
 
   err = agent_get_shadow_info_type (buf, NULL, &type);
+  xfree (buf);
   if (err)
     return 0;
-  xfree (buf);
 
   err = strcmp (type, "tpm2-v1") == 0;
   xfree (type);