vpcd: allow *extended* length APDUs

- don't predefine any limits on the transceive size - fixes out of bounds access
rude9 · Feb 6, 2018 · a63d35d · a63d35d
1 parent a9482d9
commit a63d35d
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 35 deletions.
diff --git a/virtualsmartcard/win32/BixVReader/PipeReader.cpp b/virtualsmartcard/win32/BixVReader/PipeReader.cpp
@@ -71,7 +71,7 @@ bool PipeReader::CheckATR() {
 	}
 	return true;
 }
-bool PipeReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen) {
+bool PipeReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen) {
 	//SectionLocker lock(dataSection);
 
 	if (pipe==NULL)
@@ -97,7 +97,13 @@ bool PipeReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen) {
 		pipe=NULL;
 		return false;
 	}
-	if (!ReadFile(pipe,Resp,dwRespLen,&read,NULL)) {
+	BYTE *p=(BYTE *)realloc(*Resp, dwRespLen);
+	if (p==NULL) {
+		pipe=NULL;
+		return false;
+	}
+	*Resp=p;
+	if (!ReadFile(pipe,*Resp,dwRespLen,&read,NULL)) {
 		pipe=NULL;
 		return false;
 	}

diff --git a/virtualsmartcard/win32/BixVReader/Reader.cpp b/virtualsmartcard/win32/BixVReader/Reader.cpp
@@ -199,25 +199,39 @@ void Reader::IoSmartCardTransmit(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZE_T
 	UNREFERENCED_PARAMETER(inBufSize);
 	UNREFERENCED_PARAMETER(outBufSize);
 	OutputDebugString(L"[BixVReader][TRSM]IOCTL_SMARTCARD_TRANSMIT");
-	BYTE APDU[1000];
-	int APDUSize;
-	getBuffer(pRequest,APDU,&APDUSize);
-	if (((SCARD_IO_REQUEST*)APDU)->dwProtocol!=protocol) {
+	SCARD_IO_REQUEST *scardRequest=NULL;
+	int scardRequestSize=0;
+	BYTE *RAPDU=NULL;
+	int RAPDUSize=0;
+	if (!getBuffer(pRequest,(void **)&scardRequest,&scardRequestSize)
+			|| scardRequestSize<sizeof *scardRequest
+			|| scardRequest->dwProtocol!=protocol) {
 		SectionLocker lock(device->m_RequestLock);
         pRequest->CompleteWithInformation(STATUS_INVALID_DEVICE_STATE, 0);
-		return;
+		goto end;
 	}
-	BYTE Resp[1000];
-	int RespSize;
-	if (!QueryTransmit(APDU+sizeof(SCARD_IO_REQUEST),APDUSize-sizeof(SCARD_IO_REQUEST),Resp+sizeof(SCARD_IO_REQUEST),&RespSize))
+	if (!QueryTransmit((BYTE *)(scardRequest+1),
+				scardRequestSize-sizeof(SCARD_IO_REQUEST),
+				&RAPDU,&RAPDUSize))
 	{
 		SectionLocker lock(device->m_RequestLock);
-		pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0);
-		return;
+		pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0);					
+		goto end;
 	}
-	((SCARD_IO_REQUEST*)Resp)->cbPciLength=sizeof(SCARD_IO_REQUEST);
-	((SCARD_IO_REQUEST*)Resp)->dwProtocol=protocol;
-	setBuffer(device,pRequest,Resp,RespSize+sizeof(SCARD_IO_REQUEST));
+	SCARD_IO_REQUEST *p=(SCARD_IO_REQUEST *)realloc(scardRequest,RAPDUSize+sizeof(SCARD_IO_REQUEST));
+	if (p==NULL) {
+		SectionLocker lock(device->m_RequestLock);
+        pRequest->CompleteWithInformation(STATUS_INVALID_DEVICE_STATE, 0);
+		goto end;
+	}
+	scardRequest = p;
+	scardRequest->cbPciLength=sizeof(SCARD_IO_REQUEST);
+	scardRequest->dwProtocol=protocol;
+	memcpy(scardRequest+1,RAPDU,RAPDUSize);
+	setBuffer(device,pRequest,scardRequest,RAPDUSize+sizeof(SCARD_IO_REQUEST));
+end:
+	free(scardRequest);
+	free(RAPDU);
 }
 
 void Reader::IoSmartCardGetAttribute(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZE_T outBufSize) {
@@ -345,7 +359,7 @@ bool Reader::CheckATR() {
 	return false;
 }
 
-bool Reader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen) {
+bool Reader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen) {
 	APDU;
 	APDUlen;
 	Resp;

diff --git a/virtualsmartcard/win32/BixVReader/Reader.h b/virtualsmartcard/win32/BixVReader/Reader.h
@@ -35,7 +35,7 @@ class Reader {
 	void IoSmartCardTransmit(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZE_T outBufSize);
 
 	bool initProtocols();
-	virtual bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen);
+	virtual bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen);
 	virtual bool QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset=false);
 	virtual bool CheckATR();
 	virtual DWORD startServer();
@@ -52,7 +52,7 @@ class PipeReader : public Reader {
 	HANDLE eventpipe;
 
 	PipeReader();
-	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen);
+	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen);
 	bool QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset=false);
 	bool CheckATR();
 	DWORD startServer();
@@ -74,7 +74,7 @@ class TcpIpReader : public Reader {
 	bool breakSocket;
 
 	TcpIpReader();
-	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen);
+	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen);
 	bool QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset=false);
 	bool CheckATR();
 	DWORD startServer();
@@ -95,7 +95,7 @@ class VpcdReader : public Reader {
 
 	VpcdReader();
 	~VpcdReader();
-	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen);
+	bool QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen);
 	bool QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset=false);
 	bool CheckATR();
 	DWORD startServer();

diff --git a/virtualsmartcard/win32/BixVReader/TcpIpReader.cpp b/virtualsmartcard/win32/BixVReader/TcpIpReader.cpp
@@ -45,7 +45,7 @@ bool TcpIpReader::CheckATR() {
 	}
 	return true;
 }
-bool TcpIpReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen) {
+bool TcpIpReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen) {
 	//SectionLocker lock(dataSection);
 	if (AcceptSocket==NULL)
 		return false;
@@ -73,7 +73,14 @@ bool TcpIpReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen)
 		AcceptSocket=NULL;
 		return false;
 	}
-	if ((read=recv(AcceptSocket,(char*)Resp,dwRespLen,MSG_WAITALL))<=0) {
+	BYTE *p=(BYTE *)realloc(*Resp, dwRespLen);
+	if (p==NULL) {
+		::shutdown(AcceptSocket,SD_BOTH);
+		AcceptSocket=NULL;
+		return false;
+	}
+	*Resp=p;
+	if ((read=recv(AcceptSocket,(char*)*Resp,dwRespLen,MSG_WAITALL))<=0) {
 		::shutdown(AcceptSocket,SD_BOTH);
 		AcceptSocket=NULL;
 		return false;

diff --git a/virtualsmartcard/win32/BixVReader/VpcdReader.cpp b/virtualsmartcard/win32/BixVReader/VpcdReader.cpp
@@ -44,17 +44,13 @@ bool VpcdReader::CheckATR() {
 
 	return r;
 }
-bool VpcdReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE *Resp,int *Resplen) {
-	unsigned char *rapdu = NULL;
-	ssize_t rapdu_len;
+bool VpcdReader::QueryTransmit(BYTE *APDU,int APDUlen,BYTE **Resp,int *Resplen) {
 	bool r = false;
 
-	if (APDU && APDUlen && Resp && Resplen) {
-		rapdu_len = vicc_transmit((struct vicc_ctx *) ctx, APDUlen, APDU, &rapdu);
+	if (APDU && APDUlen && Resplen) {
+		ssize_t rapdu_len = vicc_transmit((struct vicc_ctx *) ctx, APDUlen, APDU, Resp);
 		if (rapdu_len > 0) {
-			memcpy(Resp, rapdu, rapdu_len);
 			*Resplen = rapdu_len;
-			free(rapdu);
 			r = true;
 		} else {
 			signalRemoval();

diff --git a/virtualsmartcard/win32/BixVReader/memory.cpp b/virtualsmartcard/win32/BixVReader/memory.cpp
@@ -1,8 +1,7 @@
 #include "memory.h"
 #include "SectionLocker.h"
 
-bool getBuffer(IWDFIoRequest* pRequest,BYTE *buffer,int *bufferLen) {
-
+bool getBuffer(IWDFIoRequest* pRequest,void **buffer,int *bufferLen) {
 	IWDFMemory *inmem=NULL;
 	pRequest->GetInputMemory(&inmem);
 	if (inmem==NULL) {
@@ -12,13 +11,20 @@ bool getBuffer(IWDFIoRequest* pRequest,BYTE *buffer,int *bufferLen) {
 	else {
 		SIZE_T size;
 		void *data=inmem->GetDataBuffer(&size);
-		memcpy(buffer,data,size);
+		void *out = realloc(*buffer, size);
+		if (out==NULL) {
+			OutputDebugString(L"realloc failed");
+			return false;
+		}
+		memcpy(out,data,size);
 		(*bufferLen)=(int)size;
+		(*buffer)=out;
 		inmem->Release();
 		return true;
 	}
 }
-void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,BYTE *result,int inSize) {
+
+void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,int inSize) {
 	IWDFMemory *outmem=NULL;
 	pRequest->GetOutputMemory (&outmem);
 	if (outmem==NULL) {

diff --git a/virtualsmartcard/win32/BixVReader/memory.h b/virtualsmartcard/win32/BixVReader/memory.h
@@ -3,8 +3,8 @@
 #include "device.h"
 
 
-bool getBuffer(IWDFIoRequest* pRequest,BYTE *buffer,int *bufferLen);
+bool getBuffer(IWDFIoRequest* pRequest,void **buffer,int *bufferLen);
 void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,int outSize);
-void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,BYTE *result,int inSize);
+void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,int inSize);
 void setInt(CMyDevice *device,IWDFIoRequest* pRequest,DWORD result);
 DWORD getInt(IWDFIoRequest* pRequest);