pinentry-gtk-2 returns an empty passphrase string if typed passphrase is longer than 32 chars
Closed, ResolvedPublic

Description

from https://bugs.debian.org/787690:

using pinentry-gtk-2 0.9.3, typed passphrases > 32 characters return an "OK"
response with no pin.

I've tested with 0.9.2, and this bug is not present there.

this bug is also not present in 0.9.3 for the -curses, -tty, -gnome3, or -qt4
pinentry's, so it's gtk-2 and 0.9.3 specific.

Also, if i paste a long passphrase into pinentry-gtk-2, it appears to work fine.

Details

External Link
https://bugs.debian.org/787690
Version
0.9.3
dkg set Version to 0.9.3.
dkg added a subscriber: dkg.
dkg added a comment.Jun 5 2015, 3:35 AM

Tracking this down further, it appears to be caused by
1d3583a2562e83496ac515276e9bd63a7f1abbc7.

If i revert that commit, the problem goes away.

This makes me think something is wrong with secmem_realloc or secmem_malloc.

dkg assigned this task to neal.Jun 5 2015, 4:11 AM
dkg added a subscriber: neal.
dkg added a comment.Jun 5 2015, 4:56 AM

OK, something is definitely wrong with the secmem allocators.

I applied this patch:

diff --git a/secmem/secmem.c b/secmem/secmem.c
index 9a478cf..bf97a2a 100644

  • a/secmem/secmem.c

+++ b/secmem/secmem.c
@@ -381,11 +381,16 @@ secmem_realloc( void *p, size_t newsize )

mb = (MEMBLOCK*)((char*)p - ((size_t) &((MEMBLOCK*)0)->u.aligned.c));
size = mb->size;

+ printf("A: %d\n", mb->size);

    if( newsize < size )
	return p; /* it is easier not to shrink the memory */

+ printf("B: %d\n", mb->size);

a = secmem_malloc( newsize );

+ printf("C: %d\n", mb->size);

memcpy(a, p, size);

+ printf("D: %d\n", mb->size);

memset((char*)a+size, 0, newsize-size);

+ printf("E: %d\n", mb->size);

secmem_free(p);
return a;

}

and ran pinentry-gtk-2 with "getpin" as an input and typed in 32 characters for
the dialog box. at character 16, it printed:

A: 32
B: 32
C: 32
D: 32
E: 32

and at character 32 it printed:

A: 0
B: 0
C: 0
D: 0
E: 0

I'm beginning to suspect that this allocator never worked quite right, and that
1d3583a2562e83496ac515276e9bd63a7f1abbc7 just exposes a flaw in the addressing.

neal added a comment.Jun 5 2015, 5:02 AM

I've been debugging this issue for about an hour and I tentatively came to the
same conclusion.

neal added a comment.Jun 5 2015, 5:32 AM

Well, that's embarrassing. It looks like it was my bug. The attached patch
seems to fix the problem.

neal set External Link to https://bugs.debian.org/787690.Jun 5 2015, 5:34 AM
dkg added a comment.Jun 5 2015, 5:46 AM

ah, right! the other option is to pass mb->size instead of size in the memset call.

We should really synchronize secmem.c between libgcrypt:src/secmem.c,
pinentry:secmem/secmem.c, and gpg-STABLE-BRANCH-1-4:util/secmem.c :/

neal added a comment.Jun 5 2015, 5:18 PM

I've now applied the patch.

neal closed this task as Resolved.Jun 5 2015, 5:18 PM
Unknown Object (User) removed neal as the assignee of this task.May 23 2017, 4:03 AM
Unknown Object (User) updated the task description. (Show Details)
Unknown Object (User) removed External Link.
marcus assigned this task to neal.May 23 2017, 9:40 AM
marcus set External Link to https://bugs.debian.org/787690.