ssh: Fix allocation of pinentry buffer

NIIBE Yutaka gniibe at fsij.org
Mon Sep 28 10:54:35 CEST 2015


Hello,

This is a possible fix for Issue 2111 & 2112, but I know it is yet
wrong for alignment.

The bug has been around, and I think that the recent change (of
introducing the macro MAX_PASSPHRASE_LEN=255) reveals the bug.

Allocation of PI is correct.  I fixed adding - 1 because the structure
has pin[1].

The bug was adding (sizeof *pi + MAX_PASSPHRASE_LEN + 1).  The
intention was apparently adding (sizeof *pi + MAX_PASSPHRASE_LEN + 1)-byte,
but it's (sizeof *pi + MAX_PASSPHRASE_LEN + 1)* sizeof (struct pin_entry_info_s).

I changed the pointer calculation using the cast to (char *), but it
is wrong still, because it doesn't care about alignment of the
structure.  It should be rounded up to the alignment.

I have to leave now, I'll be back tomorrow.


diff --git a/agent/command-ssh.c b/agent/command-ssh.c
index 8be1255..90d9dca 100644
--- a/agent/command-ssh.c
+++ b/agent/command-ssh.c
@@ -3101,13 +3101,14 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec,
       goto out;
     }

-  pi = gcry_calloc_secure (2, sizeof (*pi) + MAX_PASSPHRASE_LEN + 1);
+  pi = gcry_calloc_secure (2, sizeof (*pi) - 1 + MAX_PASSPHRASE_LEN + 1);
   if (!pi)
     {
       err = gpg_error_from_syserror ();
       goto out;
     }
-  pi2 = pi + (sizeof *pi + MAX_PASSPHRASE_LEN + 1);
+  pi2 = (struct pin_entry_info_s *)((char *)pi + (sizeof (*pi) - 1
+                                                  + MAX_PASSPHRASE_LEN + 1));
   pi->max_length = MAX_PASSPHRASE_LEN + 1;
   pi->max_tries = 1;
   pi->with_repeat = 1;
-- 



More information about the Gnupg-devel mailing list