ssh: Fix allocation of pinentry buffer

Neal H. Walfield neal at walfield.org
Wed Sep 30 10:48:16 CEST 2015


Hi Niibe,

At Tue, 29 Sep 2015 11:08:32 +0900,
NIIBE Yutaka wrote:
> 
> On 09/28/2015 07:41 PM, Werner Koch wrote:
> > On Mon, 28 Sep 2015 10:54, gniibe at fsij.org said:
> > 
> >> 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.
> > 
> > Yep.  We better do separate mallocs for PI and PI2.  Overoptimizing
> > things is never good.
> 
> I did consider your suggestion of: two allocations and two free-s, as
> well as changing the structure itself (to have the field:
> pin[MAX_PASSPHRASE_LEN + 1]).
> 
> I realized that sizeof operator cares alignment.  Given the situation
> MAX_PASSPHRASE_LEN==255, following patch is minimum change.
> 
> I think that this is good for the fix for minimum impact.  I added a
> comment at MAX_PASSPHRASE_LEN.

I think your patch is correct, but I'd prefer a change that increases
robustness rather than one that preserves this fragility.  Second,
this bug is also present in agent/genkey.c.  Can you fix it there as
well?

Thanks,

Neal



More information about the Gnupg-devel mailing list