SCardListReaders() and non-initialized pcchReaders parameter

Sample C sample

We start with the following sample.c program:

01: #include <stdio.h>
02: #include <stdlib.h>
03: #include <winscard.h>
04: 
05: #define CHECK(f, rv) \
06:  if (SCARD_S_SUCCESS != rv) \
07:  { \
08:   printf(f ": %s\n", pcsc_stringify_error(rv)); \
09:   return -1; \
10:  }
11: 
12: int main(void)
13: {
14:     LONG rv;
15: 
16:     SCARDCONTEXT hContext;
17:     LPTSTR mszReaders;
18:     DWORD dwReaders;
19: 
20:     rv = SCardEstablishContext(SCARD_SCOPE_SYSTEM, NULL, NULL, &hContext);
21:     CHECK("SCardEstablishContext", rv)
22: 
23:     rv = SCardListReaders(hContext, NULL, NULL, &dwReaders);
24:     CHECK("SCardListReaders", rv)
25: 
26:     mszReaders = calloc(dwReaders, sizeof(char));
27:     rv = SCardListReaders(hContext, NULL, mszReaders, &dwReaders);
28:     CHECK("SCardListReaders", rv)
29:     printf("1st reader name: %s\n", mszReaders);
30: 
31:     free(mszReaders);
32: 
33:     rv = SCardReleaseContext(hContext);
34:     CHECK("SCardReleaseContext", rv)
35: 
36:     return 0;
37: }
38: 

Makefile

I used this Makefile

# Linux
PCSC_CFLAGS := $(shell pkg-config --cflags libpcsclite)
LDLIBS := $(shell pkg-config --libs libpcsclite)

CFLAGS = $(PCSC_CFLAGS) -g

sample: sample.c

clean:
	rm -f sample

Output

Compilation and execution give:

$ make
cc -pthread -I/usr/include/PCSC -g   sample.c  -lpcsclite -o sample
$ ./sample 
1st reader name: Alcor Micro AU9540 00 00

It looks like everything is correct.

But can you find the problem?

 

Valgrind is your friend

valgrind is a very nice tool to debug issues.

Valgrind is an instrumentation framework for building dynamic analysis tools. There are Valgrind tools that can automatically detect many memory management and threading bugs, and profile your programs in detail. You can also use Valgrind to build new tools.

valgrind sees a problem:

$ valgrind --track-origins=yes ./sample 

==99878== Memcheck, a memory error detector
==99878== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==99878== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==99878== Command: ./sample
==99878== 
==99878== Conditional jump or move depends on uninitialised value(s)
==99878==    at 0x486E02E: SCardListReaders (in /usr/lib/x86_64-linux-gnu/libpcsclite.so.1.0.0)
==99878==    by 0x109210: main (sample.c:23)
==99878==  Uninitialised value was created by a stack allocation
==99878==    at 0x109199: main (sample.c:13)
==99878== 
1st reader name: Alcor Micro AU9540 00 00
==99878== 
==99878== HEAP SUMMARY:
==99878==     in use at exit: 112 bytes in 4 blocks
==99878==   total heap usage: 10 allocs, 6 frees, 1,434 bytes allocated
==99878== 
==99878== LEAK SUMMARY:
==99878==    definitely lost: 0 bytes in 0 blocks
==99878==    indirectly lost: 0 bytes in 0 blocks
==99878==      possibly lost: 0 bytes in 0 blocks
==99878==    still reachable: 112 bytes in 4 blocks
==99878==         suppressed: 0 bytes in 0 blocks
==99878== Rerun with --leak-check=full to see details of leaked memory
==99878== 
==99878== For lists of detected and suppressed errors, rerun with: -s
==99878== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Using uninitialized variables is bad. It creates bugs that declare at random time and are not easy to find.

SCardListReaders

The problem is at line 23 which is:

23:     rv = SCardListReaders(hContext, NULL, NULL, &dwReaders);

If we read the documentation for SCardListReaders() we have:

Returns a list of currently available readers on the system.

mszReaders is a pointer to a character string that is allocated by the application. If the application sends mszGroups and mszReaders as NULL then this function will return the size of the buffer needed to allocate in pcchReaders.

If *pcchReaders is equal to SCARD_AUTOALLOCATE then the function will allocate itself the needed memory. Use SCardFreeMemory() to release it.

Parameters

[in]	hContext	Connection context to the PC/SC Resource Manager.
[in]	mszGroups	List of groups to list readers (not used).
[out]	mszReaders	Multi-string with list of readers.
[in,out]	pcchReaders	Size of multi-string buffer including NULL's.

You can note that the parameter pcchReaders is in and out. This is because the value is compared to SCARD_AUTOALLOCATE. So the value of pcchReaders shall be set before calling SCardListReaders().

The fix is simple. Just update line 18 like this:

18: DWORD dwReaders = 0;

 

Looking for problems

What happens if we initialize dwReaders to the special value SCARD_AUTOALLOCATE instead of 0?

18: DWORD dwReaders = SCARD_AUTOALLOCATE;

We build and run the sample:

$ ./sample 
SCardListReaders: Invalid parameter given.

The execution of SCardListReaders() fails.

23:     rv = SCardListReaders(hContext, NULL, NULL, &dwReaders);

This is because we are asking SCardListReaders() to allocate and store the result in the parameter mszReaders. But in our sample this parameter is NULL. pcsc-lite has a check for that and returns an error code.

Instead of using NULL we could use something else like 0x1234.

23:     rv = SCardListReaders(hContext, NULL, 0x1234, &dwReaders);

This time we have a nice crash:

$ ./sample 
Segmentation fault

This is because we asked SCardListReaders() to write at the address 0x1234. This address is, in general, not valid.

This problem could happen if you use something like:

23:     rv = SCardListReaders(hContext, NULL, mszReaders, &dwReaders);

And both variables mszReaders and dwReaders are uninitialized, and by a total lack of luck dwReaders has the value SCARD_AUTOALLOCATE (i.e. -1).

 

GnuPG

The problem was found in by oddlama and reported to PCSC at "pcscd fails to read future yubikeys after removing a yubikey, until restarted #125".

The problem is not in pcsc-lite but in GnugPG. So I reported the problem at "SCardListReaders: Conditional jump or move depends on uninitialised value(s)".

WinSCard API

macOS does not support (yet) the value SCARD_AUTOALLOCATE. But pcsc-lite and Windows WinSCard do.

I agree the API to use SCARD_AUTOALLOCATE is ugly. We pass the address of a buffer pointer in a parameter that is a buffer pointer. We have to cast the variable like I did in the C sample like:

  dwReaders = SCARD_AUTOALLOCATE;
  rv = SCardListReaders(hContext, NULL, (LPTSTR)&mszReaders, &dwReaders);

This feature is a Microsoft extension that is not present in the PCSC workgroup specification.

Conclusion

I think the problem is not very known and should be better documented. That was my motivation for writting this blog article.

Morality: in C language, always initialize your variables to a known/safe value (like 0).