Hi,
i found a bug in the gpg smartcard code in g10/apdu.c - i have attached a
detailed description and a patch, as well as a small conclusion at the end.
Currently, one can't actually use gpg 1.4.11 with a smartcard reader in 64bit
environments. The reason for the current error is the usage of an uninitialized
64bit variable that is processed (as 32bit value) by PC/SC library and passed to
the memory allocator afterwards.
when trying to sign a file, gpg stops with a malloc error:
iiii:g10 ths$ gpg -s -u 0x98459AB6 lala.txt
gpg(34669,0x7fff70d76cc0) malloc: *** mmap(size=140733193392128) failed (error
code=12)
- error: can't allocate region
- set a breakpoint in malloc_error_break to debug
gpg: out of memory while allocating 36 bytes
This happens, if the resulting file (lala.txt.gpg or lala.txt.asc for --
clearsig) does NOT exist. If the file already exists, gpg asks whether to
overwrite or not the existing result-file, and the signature generation works
just as expected:
iiii:g10 ths$ gpg -s -u 0x98459AB6 lala.txt
File `lala.txt.gpg' exists. Overwrite? (y/N) y
gpg: detected reader `REINER SCT cyberJack ecom_a 00 00'
gpg: signatures created so far: 42
Please enter the PIN
[sigs done: 42]
Enter PIN:
...
I recompiled with -g and attached gdb:
---snip---
Breakpoint 1, 0x00007fff8730c499 in malloc_error_break ()
(gdb) bt
#0 0x00007fff8730c499 in malloc_error_break ()
#1 0x00007fff8730d5f0 in szone_error ()
#2 0x00007fff87232876 in allocate_pages ()
#3 0x00007fff87242951 in large_malloc ()
#4 0x00007fff87234709 in szone_malloc_should_clear ()
#5 0x00007fff8723398a in malloc_zone_malloc ()
#6 0x00007fff87231c88 in malloc ()
#7 0x00000001000a4f97 in xmalloc (n=140733193388068) at memory.c:443
#8 0x00000001000413eb in apdu_open_reader (portstr=0x0) at apdu.c:1499
#9 0x0000000100031038 in open_card () at cardglue.c:453
#10 0x000000010003182e in agent_scd_pksign (serialno=0x10040cf20
"D276000124010200000500000BFE0000/20B244DE253781693E0597FACF20D03F98459AB6",
hashalgo=2, indata=0x10040d188 "\035>䁅\"I??9??9\002???\022o", indatalen=20,
r_buf=0x7fff5fbff098, r_buflen=0x7fff5fbff090) at cardglue.c:1302
#11 0x0000000100051d6b in do_sign (sk=0x100409aa0, sig=0x10040e3f0,
md=0x100824400, digest_algo=2) at sign.c:270
#12 0x0000000100051f75 in write_signature_packets (sk_list=<value temporarily
unavailable, due to optimizations>, out=0x10040cfa0, hash=0x100824000, sigclass=
<value temporarily unavailable, due to optimizations>, timestamp=1310741566,
duration=0, status_letter=67) at sign.c:669
#13 0x0000000100052cb9 in clearsign_file (fname=<value temporarily unavailable,
due to optimizations>, locusr=<value temporarily unavailable, due to
optimizations>, outfile=0x1 <Address 0x1 out of bounds>) at sign.c:1171
#14 0x0000000100009509 in main (argc=1, argv=0x7fff5fbff9b8) at gpg.c:3501
(gdb) frame 7
#7 0x00000001000a4f97 in xmalloc (n=140733193388068) at memory.c:443
443 if( !(p = malloc( n )) )
(gdb) list
438 #else
439 /* mallocing zero bytes is undefined by ISO-C, so we better make
440 sure that it won't happen */
441 if (!n)
442 n = 1;
443 if( !(p = malloc( n )) )
444 out_of_core(n,0);
445 return p;
446 #endif
447 }
(gdb)
---snip---
The size looks quite fishy to me (0x7fff00001000)...
let's see what's happening here:
#8 0x00000001000413eb in apdu_open_reader (portstr=0x0) at apdu.c:1499
---snip---
err = pcsc_list_readers (reader_table[slot].pcsc.context,
NULL, NULL, &nreader);
if (!err)
{line 1499: list = xtrymalloc (nreader+1); /* Better add 1 for safety reasons.
*/
---snip---
nreader is defined at apdu.c:1476
---snip---
unsigned long nreader, listlen;
---snip---
it is a NOT initialized 64bit integer, its initial value is _undefinied_.
A pointer to nreader is passed to pcsc_list_readers() at line 1495 which is a
function pointer to the pcsc library: (line 254):
---snip---
long (* DLSTDCALL pcsc_list_readers) (unsigned long context,
const char *groups,
char *readers, unsigned long*readerslen);
^^^^^^^^^^^^^^^^^^[...]
/* No ctAPI configured, so lets try the PC/SC API */
[...]
pcsc_list_readers = dlsym (handle, "SCardListReaders");
---snip---
The pcsc-lite function SCardListReaders() on the other hand is defined &
implemented as:
(see http://pcsclite.alioth.debian.org/api/winscard__clnt_8c_source.html)
---snip---
02945 LONG SCardListReaders(SCARDCONTEXT hContext, /*@unused@*/ LPCSTR
mszGroups,
02946 LPSTR mszReaders, LPDWORD pcchReaders)
^^^^^^^^^^^^^^^^^^^^
02947 {
02948 DWORD dwReadersLen = 0;
^^^^^^^^^^^^ ^^^^^^
[...]
03049 /* set the reader names length */
03050 *pcchReaders = dwReadersLen;
[...]
---snip---
DWORD is defined as a 32bit integer in /usr/include/iodbcunix.h
typedef unsigned int DWORD;
typedef DWORD * LPDWORD;
Conclusion:
The undefined, initial value of nreaders was larger than 0xFFFFFFFF (at least
0x7FFF00000000). SCardListReaders() assigned the readers-length to the least
significant 32bit of nreaders (due to the implicit integer narrowing). The
caller passes nreaders (thus having the value 0x7FFF00001000) to malloc() which
actually fails. If i had enough RAM, this would be at least a memory exhaustion
bug ;) Since the error message printf() also truncates the actual size value, it
is not obvious what actually happened, since it claims not being able to
allocate 36 Byte (one page a 4k) of RAM.
Patch:
---snip---
- g10/apdu.c.orig 2011-07-15 17:04:32.000000000 +0200
+++ g10/apdu.c 2011-07-15 17:49:52.000000000 +0200
@@ -1473,7 +1473,7 @@
long err; int slot; char *list = NULL;
- unsigned long nreader, listlen;
+ unsigned long nreader=0, listlen=0;
char *p; slot = new_reader_slot ();
---snip---
It might be better, to use DWORD/unsigned int nreaders = 0 instead of an
unsigned long.
I'd also like to ask, if you please could explain in depth your comment in line
1499: "Better add 1 for safety reasons." - what is the actual benefit of this
code +1?
Best regards,
Thorsten
Environment: Darwin xxxx 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun 7
16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64 x86_64