agent: Fix alignment problem with the second passphrase struct.
authorWerner Koch <wk@gnupg.org>
Thu, 1 Oct 2015 11:21:25 +0000 (13:21 +0200)
committerNIIBE Yutaka <gniibe@fsij.org>
Mon, 5 Oct 2015 17:11:00 +0000 (02:11 +0900)
* agent/genkey.c (agent_ask_new_passphrase): Use a separate malloc for
PI2.  Check return value of the malloc function.
* agent/command-ssh.c (ssh_identity_register): Use a separate malloc
for PI2.  Wipe PI2.
--

For whatever stupid reasons I once allocated only one memory area and
split that into PI and PI2.  This is actually a common pattern with
malloc but here we used a made up object size and do not take the
extra alignment required into account.  One of these not yet hit by
a (sig)bus PC/VAX hacker bugs.

Instead of trying to fix the alignment, it is better to use a second
calloc for the second struct.

GnuPG-bug-id: 2112
Signed-off-by: Werner Koch <wk@gnupg.org>
Resolved conflicts:
agent/command-ssh.c
agent/genkey.c

(backport master commit ddf9dd135acd2b3635bb986f6dfc0e4e446d5fad)

agent/command-ssh.c
agent/genkey.c

index 2aacecc..0a12bb6 100644 (file)
@@ -2835,7 +2835,8 @@ ssh_identity_register (ctrl_t ctrl, gcry_sexp_t key, int ttl, int confirm)
   char *key_fpr = NULL;
   const char *initial_errtext = NULL;
   unsigned int i;
-  struct pin_entry_info_s *pi = NULL, *pi2;
+  struct pin_entry_info_s *pi = NULL;
+  struct pin_entry_info_s *pi2 = NULL;
 
   err = ssh_key_grip (key, key_grip_raw);
   if (err)
@@ -2866,16 +2867,21 @@ ssh_identity_register (ctrl_t ctrl, gcry_sexp_t key, int ttl, int confirm)
       goto out;
     }
 
-  pi = gcry_calloc_secure (2, sizeof (*pi) + 100 + 1);
+  pi = gcry_calloc_secure (1, sizeof (*pi) + 100 + 1);
   if (!pi)
     {
       err = gpg_error_from_syserror ();
       goto out;
     }
-  pi2 = pi + (sizeof *pi + 100 + 1);
-  pi->max_length = 100;
+  pi->max_length = 100 + 1;
   pi->max_tries = 1;
-  pi2->max_length = 100;
+  pi2 = gcry_calloc_secure (1, sizeof (*pi2) + 100 + 1);
+  if (!pi2)
+    {
+      err = gpg_error_from_syserror ();
+      goto out;
+    }
+  pi2->max_length = 100 + 1;
   pi2->max_tries = 1;
   pi2->check_cb = reenter_compare_cb;
   pi2->check_cb_arg = pi->pin;
@@ -2920,6 +2926,9 @@ ssh_identity_register (ctrl_t ctrl, gcry_sexp_t key, int ttl, int confirm)
 
 
  out:
+  if (pi2 && pi2->max_length)
+    wipememory (pi2->pin, pi2->max_length);
+  xfree (pi2);
   if (pi && pi->max_length)
     wipememory (pi->pin, pi->max_length);
   xfree (pi);
index 65477ad..562f7a2 100644 (file)
@@ -310,12 +310,12 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
     const char *text2 = _("Please re-enter this passphrase");
     const char *initial_errtext = NULL;
 
-    pi = gcry_calloc_secure (2, sizeof (*pi) + 100);
-    pi2 = pi + (sizeof *pi + 100);
-    pi->max_length = 100;
+    pi = gcry_calloc_secure (1, sizeof (*pi) + 100 + 1);
+    pi->max_length = 100 + 1;
     pi->max_tries = 3;
     pi->with_qualitybar = 1;
-    pi2->max_length = 100;
+    pi2 = gcry_calloc_secure (1, sizeof (*pi) + 100 + 1);
+    pi2->max_length = 100 + 1;
     pi2->max_tries = 3;
     pi2->check_cb = reenter_compare_cb;
     pi2->check_cb_arg = pi->pin;
@@ -345,13 +345,15 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
     if (rc)
       {
         xfree (pi);
+        xfree (pi2);
         return rc;
       }
 
     if (!*pi->pin)
       {
         xfree (pi);
-        pi = NULL; /* User does not want a passphrase. */
+        xfree (pi2);
+        pi = pi2 = NULL; /* User does not want a passphrase. */
       }
   }
 
@@ -361,6 +363,7 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
     {
       log_error ("key generation failed: %s\n", gpg_strerror (rc));
       xfree (pi);
+      xfree (pi2);
       return rc;
     }
 
@@ -371,6 +374,7 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
       log_error ("key generation failed: invalid return value\n");
       gcry_sexp_release (s_key);
       xfree (pi);
+      xfree (pi2);
       return gpg_error (GPG_ERR_INV_DATA);
     }
   s_public = gcry_sexp_find_token (s_key, "public-key", 0);
@@ -380,6 +384,7 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
       gcry_sexp_release (s_private);
       gcry_sexp_release (s_key);
       xfree (pi);
+      xfree (pi2);
       return gpg_error (GPG_ERR_INV_DATA);
     }
   gcry_sexp_release (s_key); s_key = NULL;
@@ -388,7 +393,8 @@ agent_genkey (ctrl_t ctrl, const char *keyparam, size_t keyparamlen,
   if (DBG_CRYPTO)
     log_debug ("storing private key\n");
   rc = store_key (s_private, pi? pi->pin:NULL, 0);
-  xfree (pi); pi = NULL;
+  xfree (pi);
+  xfree (pi2);
   gcry_sexp_release (s_private);
   if (rc)
     {
@@ -432,12 +438,12 @@ agent_protect_and_store (ctrl_t ctrl, gcry_sexp_t s_skey)
     const char *text2 = _("Please re-enter this passphrase");
     const char *initial_errtext = NULL;
 
-    pi = gcry_calloc_secure (2, sizeof (*pi) + 100);
-    pi2 = pi + (sizeof *pi + 100);
-    pi->max_length = 100;
+    pi = gcry_calloc_secure (1, sizeof (*pi) + 100 + 1);
+    pi->max_length = 100 + 1;
     pi->max_tries = 3;
     pi->with_qualitybar = 1;
-    pi2->max_length = 100;
+    pi2 = gcry_calloc_secure (1, sizeof (*pi) + 100 + 1);
+    pi2->max_length = 100 + 1;
     pi2->max_tries = 3;
     pi2->check_cb = reenter_compare_cb;
     pi2->check_cb_arg = pi->pin;
@@ -468,17 +474,20 @@ agent_protect_and_store (ctrl_t ctrl, gcry_sexp_t s_skey)
     if (rc)
       {
         xfree (pi);
+        xfree (pi2);
         return rc;
       }
 
     if (!*pi->pin)
       {
         xfree (pi);
-        pi = NULL; /* User does not want a passphrase. */
+        xfree (pi2);
+        pi = pi2 = NULL; /* User does not want a passphrase. */
       }
   }
 
   rc = store_key (s_skey, pi? pi->pin:NULL, 1);
   xfree (pi);
+  xfree (pi2);
   return rc;
 }