On Tue, 2017-01-17 at 20:33 +0100, Maksim Ivanov wrote: > The pcsc_stringify_error function in the PC/SC-Lite implementation > uses a statically allocated buffer. This means that the buffer may be > used simultaneously when the function is called from multiple threads > concurrently. > Therefore, the returned message may be spoiled, e.g.: > "Internal error.ul" > or > "Command cancell" > In the worst-case scenario, the application may read an unbounded > string (with the terminating null character missing).
A possible fix is attached. That avoids copying strings which are constant on global store, and ensures that the static buffer is on thread local store when possible. Except compilation, the fix is completely untested. regards, Nikos
From 27d52931d9fc78725fe451008cabdedb0dbe6d44 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos <[email protected]> Date: Wed, 18 Jan 2017 11:25:25 +0100 Subject: [PATCH] pcsc_stringify_error: address overlapping static variables issues This is addressed in two steps. First do not copy a constant static string into a static shared buffer unless it is necessary. This ensures that constant strings will not overwrite themselves when the function is called from multiple threads. In addition set the static buffers in the thread local storage on platforms where this is possible. Signed-off-by: Nikos Mavrogiannopoulos <[email protected]> --- configure.ac | 2 +- src/PCSC/pcsclite.h.in | 2 +- src/error.c | 28 +++++++++++++++++++--------- src/spy/libpcscspy.c | 2 +- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/configure.ac b/configure.ac index 8eea5e3..028aca4 100644 --- a/configure.ac +++ b/configure.ac @@ -94,7 +94,7 @@ LT_INIT # Checks for header files AC_HEADER_STDC AC_HEADER_SYS_WAIT -AC_CHECK_HEADERS([getopt.h sys/filio.h syslog.h dl.h fcntl.h alloca.h]) +AC_CHECK_HEADERS([getopt.h sys/filio.h syslog.h dl.h fcntl.h alloca.h thread.h]) # Checks for typedefs, structures, and compiler characteristics AC_C_CONST diff --git a/src/PCSC/pcsclite.h.in b/src/PCSC/pcsclite.h.in index 5b3547e..83a17dd 100644 --- a/src/PCSC/pcsclite.h.in +++ b/src/PCSC/pcsclite.h.in @@ -300,7 +300,7 @@ extern const SCARD_IO_REQUEST g_rgSCardT0Pci, g_rgSCardT1Pci, g_rgSCardRawPci; /* * Gets a stringified error response */ -char *pcsc_stringify_error(const LONG); +const char *pcsc_stringify_error(const LONG); #ifdef __cplusplus } diff --git a/src/error.c b/src/error.c index 08db0ca..30e83ab 100644 --- a/src/error.c +++ b/src/error.c @@ -47,10 +47,19 @@ THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "pcsclite.h" #include "string.h" +#if defined(HAVE_THREADS_H) +# include <threads.h> +#elif defined(__GNUC__) +# define _Thread_local __thread +#else +# define _Thread_local +#endif + #ifdef NO_LOG +const PCSC_API char* pcsc_stringify_error(const LONG pcscError) { - static char strError[] = "0x12345678"; + static _Thread_local char strError[] = "0x12345678"; snprintf(strError, sizeof(strError), "0x%08lX", pcscError); @@ -74,9 +83,10 @@ PCSC_API char* pcsc_stringify_error(const LONG pcscError) * pcsc_stringify_error(rv), rv); * @endcode */ +const PCSC_API char* pcsc_stringify_error(const LONG pcscError) { - static char strError[75]; + static _Thread_local char strError[75]; const char *msg = NULL; switch (pcscError) @@ -221,16 +231,16 @@ PCSC_API char* pcsc_stringify_error(const LONG pcscError) pcscError); }; - if (msg) - (void)strncpy(strError, msg, sizeof(strError)); - else + if (msg) { + return msg; + } else { (void)snprintf(strError, sizeof(strError)-1, "Unknown error: 0x%08lX", pcscError); + /* add a null byte */ + strError[sizeof(strError)-1] = '\0'; - /* add a null byte */ - strError[sizeof(strError)-1] = '\0'; - - return strError; + return strError; + } } #endif diff --git a/src/spy/libpcscspy.c b/src/spy/libpcscspy.c index 80f9009..525150a 100644 --- a/src/spy/libpcscspy.c +++ b/src/spy/libpcscspy.c @@ -78,7 +78,7 @@ PCSC_API int32_t SCardControl132(SCARDHANDLE hCard, uint32_t dwControlCode, #define p_SCardSetAttrib(fct) LONG(fct) (SCARDHANDLE hCard, DWORD dwAttrId, LPCBYTE pbAttr, DWORD cbAttrLen) -#define p_pcsc_stringify_error(fct) char *(fct)(const LONG pcscError) +#define p_pcsc_stringify_error(fct) const char *(fct)(const LONG pcscError) /* fake function to just return en error code */ static LONG internal_error(void) -- 2.9.3
_______________________________________________ Pcsclite-muscle mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
