From caa555a5bfaa98f8f630901427a653bd8dc7b95e Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 1 Oct 2015 13:21:25 +0200 Subject: [PATCH] agent: Fix alignment problem with the second passphrase struct. * 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 Resolved conflicts: agent/command-ssh.c agent/genkey.c (backport master commit ddf9dd135acd2b3635bb986f6dfc0e4e446d5fad) --- agent/command-ssh.c | 19 ++++++++++++++----- agent/genkey.c | 31 ++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/agent/command-ssh.c b/agent/command-ssh.c index 2aacecc46..0a12bb676 100644 --- a/agent/command-ssh.c +++ b/agent/command-ssh.c @@ -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); diff --git a/agent/genkey.c b/agent/genkey.c index 65477adf2..562f7a2d2 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -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; }