[Pcsclite-git-commit] [PCSC] 01/01: SCardTransmit() may return SCARD_E_INSUFFICIENT_BUFFER

[Ludovic Rousseau]

This is an automated email from the git hooks/post-receive script.

rousseau pushed a commit to branch master
in repository PCSC.

commit 8eb9ea1b354b050f997d003cf3b0c5b56f29f9f7
Author: Ludovic Rousseau <ludovic.rousseau at gmail.com>
Date:   Fri May 29 10:01:43 2015 +0200

    SCardTransmit() may return SCARD_E_INSUFFICIENT_BUFFER
    
    SCardTransmit() now correctly returns SCARD_E_INSUFFICIENT_BUFFER when
    pcbRecvLength is not big enough to receive the card response.
    
    The APDU is succesfully exchanged with the card but the card response is
    lost since the client application buffer is not large enough to receive
    it.
    
    Before this patch an error was reported in the CCID driver and the
    application got a SCARD_E_NOT_TRANSACTED error instead.
    
    Now the expected size is available in pcbRecvLength for the application
    to adapt its buffer size and, possibly, recall SCardTransmit().
    
    Thanks to Mironenko for the bug report
    https://github.com/LudovicRousseau/CCID/pull/5
---
 UnitaryTests/BufferOverflow2.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 UnitaryTests/Makefile          |  1 +
 src/winscard_clnt.c            |  2 +-
 src/winscard_svc.c             |  8 +++-
 4 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/UnitaryTests/BufferOverflow2.c b/UnitaryTests/BufferOverflow2.c
new file mode 100644
index 0000000..ad463d4
--- /dev/null
+++ b/UnitaryTests/BufferOverflow2.c
@@ -0,0 +1,86 @@
+/*
+ * MUSCLE SmartCard Development ( http://pcsclite.alioth.debian.org/pcsclite.html )
+ *
+ * Copyright (C) 2009
+ *  Ludovic Rousseau <ludovic.rousseau at free.fr>
+ */
+#include <stdio.h>
+#include <string.h>
+
+#ifdef __APPLE__
+#include <PCSC/winscard.h>
+#include <PCSC/wintypes.h>
+#else
+#include <winscard.h>
+#endif
+
+#define GREEN "\33[32m"
+#define BRIGHT_RED "\33[01;31m"
+#define NORMAL "\33[0m"
+
+int main(void)
+{
+	SCARDCONTEXT hContext;
+	SCARDHANDLE hCard;
+	DWORD dwActiveProtocol;
+	LONG rv;
+	char mszReaders[1024];
+	DWORD dwReaders = sizeof(mszReaders);
+	SCARD_IO_REQUEST ioRecvPci = *SCARD_PCI_T0;	/* use a default value */
+	unsigned char bSendBuffer[MAX_BUFFER_SIZE];
+	unsigned char bRecvBuffer[MAX_BUFFER_SIZE];
+	DWORD send_length, length;
+
+	rv = SCardEstablishContext(SCARD_SCOPE_SYSTEM, NULL, NULL, &hContext);
+	printf("SCardEstablishContext %lX\n", rv);
+
+	rv = SCardListReaders(hContext, NULL, mszReaders, &dwReaders);
+	printf("SCardListReaders %lX\n", rv);
+
+	rv = SCardConnect(hContext, mszReaders, SCARD_SHARE_SHARED,
+		SCARD_PROTOCOL_T0 | SCARD_PROTOCOL_T1, &hCard,
+		&dwActiveProtocol);
+	printf("SCardConnect %lX\n", rv);
+
+	send_length = 5;
+	/* GET RANDOM for a GPK card */
+	memcpy(bSendBuffer, "\x80\x84\x00\x00\x20", send_length);
+
+	/* expected size is 0x20 + 2 = 34 bytes */
+	length = 30;
+	rv = SCardTransmit(hCard, SCARD_PCI_T0, bSendBuffer, send_length,
+        &ioRecvPci, bRecvBuffer, &length);
+	if (SCARD_E_INSUFFICIENT_BUFFER == rv)
+	{
+		printf(GREEN "test PASS. Insufficient buffer is expected\n" NORMAL);
+	}
+	else
+	{
+		printf(BRIGHT_RED "test FAIL\n" NORMAL);
+	}
+	printf("SCardTransmit %lX: %s\n", rv, pcsc_stringify_error(rv));
+	printf("Expected length: %ld\n", length);
+	if (SCARD_S_SUCCESS == rv)
+	{
+		int i;
+
+		for (i=0; i<length; i++)
+			printf("%02X ", bRecvBuffer[i]);
+		printf("\n");
+	}
+
+	rv = SCardTransmit(hCard, SCARD_PCI_T0, bSendBuffer, send_length,
+        &ioRecvPci, bRecvBuffer, &length);
+	printf("SCardTransmit %lX: %s\n", rv, pcsc_stringify_error(rv));
+	printf("Expected length: %ld\n", length);
+	if (SCARD_S_SUCCESS == rv)
+	{
+		int i;
+
+		for (i=0; i<length; i++)
+			printf("%02X ", bRecvBuffer[i]);
+		printf("\n");
+	}
+
+	return 0;
+}
diff --git a/UnitaryTests/Makefile b/UnitaryTests/Makefile
index 91d2cc0..cce25ff 100644
--- a/UnitaryTests/Makefile
+++ b/UnitaryTests/Makefile
@@ -11,6 +11,7 @@ LDLIBS := $(PCSC_LDLIBS)
 
 PROGRAMS := SCardBeginTransaction \
 	BufferOverflow \
+	BufferOverflow2 \
 	exec
 
 all: $(PROGRAMS)
diff --git a/src/winscard_clnt.c b/src/winscard_clnt.c
index 381eb1a..d77a0b1 100644
--- a/src/winscard_clnt.c
+++ b/src/winscard_clnt.c
@@ -2709,7 +2709,7 @@ end:
  *
  * @return Error code.
  * @retval SCARD_S_SUCCESS Successful (\ref SCARD_S_SUCCESS)
- * @retval SCARD_E_INSUFFICIENT_BUFFER \p cbSendLength or \p cbRecvLength are too big (\ref SCARD_E_INSUFFICIENT_BUFFER)
+ * @retval SCARD_E_INSUFFICIENT_BUFFER \p cbRecvLength was not large enough for the card response. The expected size is now in \p cbRecvLength (\ref SCARD_E_INSUFFICIENT_BUFFER)
  * @retval SCARD_E_INVALID_HANDLE Invalid \p hCard handle (\ref SCARD_E_INVALID_HANDLE)
  * @retval SCARD_E_INVALID_PARAMETER \p pbSendBuffer or \p pbRecvBuffer or \p pcbRecvLength or \p pioSendPci is null (\ref SCARD_E_INVALID_PARAMETER)
  * @retval SCARD_E_INVALID_VALUE Invalid Protocol, reader name, etc (\ref SCARD_E_INVALID_VALUE)
diff --git a/src/winscard_svc.c b/src/winscard_svc.c
index d5c81a6..5008ae0 100644
--- a/src/winscard_svc.c
+++ b/src/winscard_svc.c
@@ -638,12 +638,18 @@ static void ContextThread(LPVOID newContext)
 				ioSendPci.cbPciLength = trStr.ioSendPciLength;
 				ioRecvPci.dwProtocol = trStr.ioRecvPciProtocol;
 				ioRecvPci.cbPciLength = trStr.ioRecvPciLength;
-				cbRecvLength = trStr.pcbRecvLength;
+				cbRecvLength = sizeof pbRecvBuffer;
 
 				trStr.rv = SCardTransmit(trStr.hCard, &ioSendPci,
 					pbSendBuffer, trStr.cbSendLength, &ioRecvPci,
 					pbRecvBuffer, &cbRecvLength);
 
+				if (cbRecvLength > trStr.pcbRecvLength)
+					/* The client buffer is not large enough.
+					 * The pbRecvBuffer buffer will NOT be sent a few
+					 * lines bellow. So no buffer overflow is expected. */
+					trStr.rv = SCARD_E_INSUFFICIENT_BUFFER;
+
 				trStr.ioSendPciProtocol = ioSendPci.dwProtocol;
 				trStr.ioSendPciLength = ioSendPci.cbPciLength;
 				trStr.ioRecvPciProtocol = ioRecvPci.dwProtocol;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pcsclite/PCSC.git