vpcd: added more input checking (#328)
Some checks failed
Build / virtualsmartcard-macos (push) Has been cancelled
Build / virtualsmartcard-ubuntu (push) Has been cancelled
Build / ccid-ubuntu (push) Has been cancelled
Build / pcsc-relay-macos (push) Has been cancelled
Build / pcsc-relay-ubuntu (push) Has been cancelled
Build / pcsc-relay-mingw-64 (push) Has been cancelled
Build / remote-reader-ubuntu (push) Has been cancelled
Build / ACardEmulator-ubuntu (push) Has been cancelled
Coverity Scan / build (push) Has been cancelled

May fix unstability issues #326 #324

(cherry picked from commit 7c949d1ec0e40ca2aabf92192cdf9f66a4a7a0bf)
This commit is contained in:
Frank Morgner
2026-03-12 21:46:29 +01:00
committed by Christoph Honal
parent 0f84236f4a
commit d9c873dec3
6 changed files with 37 additions and 26 deletions

View File

@@ -126,6 +126,7 @@ bool PipeReader::QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset) {
} }
if (size==0) if (size==0)
return false; return false;
size=min(size,*ATRsize);
if (!ReadFile(pipe,ATR,size,&read,NULL)) { if (!ReadFile(pipe,ATR,size,&read,NULL)) {
pipe=NULL; pipe=NULL;
return false; return false;

View File

@@ -88,7 +88,7 @@ void Reader::IoSmartCardPower(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZE_T ou
} }
if (code==SCARD_COLD_RESET || code==SCARD_WARM_RESET) { if (code==SCARD_COLD_RESET || code==SCARD_WARM_RESET) {
BYTE ATR[100]; BYTE ATR[100];
DWORD ATRsize; DWORD ATRsize=sizeof(ATR);
if (!QueryATR(ATR,&ATRsize,true)) if (!QueryATR(ATR,&ATRsize,true))
{ {
pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0); pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0);
@@ -114,7 +114,7 @@ void Reader::IoSmartCardSetProtocol(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZ
OutputDebugString(log); OutputDebugString(log);
BYTE ATR[100]; BYTE ATR[100];
DWORD ATRsize; DWORD ATRsize=sizeof(ATR);
state=SCARD_SPECIFIC; state=SCARD_SPECIFIC;
if (!QueryATR(ATR,&ATRsize,true)) if (!QueryATR(ATR,&ATRsize,true))
{ {
@@ -200,7 +200,7 @@ void Reader::IoSmartCardTransmit(IWDFIoRequest* pRequest,SIZE_T inBufSize,SIZE_T
UNREFERENCED_PARAMETER(outBufSize); UNREFERENCED_PARAMETER(outBufSize);
OutputDebugString(L"[VivoKeySmartReader][TRSM]IOCTL_SMARTCARD_TRANSMIT"); OutputDebugString(L"[VivoKeySmartReader][TRSM]IOCTL_SMARTCARD_TRANSMIT");
SCARD_IO_REQUEST *scardRequest=NULL; SCARD_IO_REQUEST *scardRequest=NULL;
int scardRequestSize=0; SIZE_T scardRequestSize=0;
BYTE *RAPDU=NULL; BYTE *RAPDU=NULL;
int RAPDUSize=0; int RAPDUSize=0;
if (!getBuffer(pRequest,(void **)&scardRequest,&scardRequestSize) if (!getBuffer(pRequest,(void **)&scardRequest,&scardRequestSize)
@@ -255,8 +255,9 @@ void Reader::IoSmartCardGetAttribute(IWDFIoRequest* pRequest,SIZE_T inBufSize,SI
if (rpcType==0) { if (rpcType==0) {
PipeReader *pipe=(PipeReader *)this; PipeReader *pipe=(PipeReader *)this;
OutputDebugString(L"[VivoKeySmartReader][GATT]PIPE_NAME"); OutputDebugString(L"[VivoKeySmartReader][GATT]PIPE_NAME");
sprintf(temp,"%S",pipe->pipeName); sprintf(temp,"%.*S",(int)sizeof(temp),pipe->pipeName);
setString(device,pRequest,(char*)temp,(int)outBufSize); temp[sizeof(temp)-1] = '\0';
setString(device,pRequest,(char*)temp,outBufSize);
} }
else { else {
SectionLocker lock(device->m_RequestLock); SectionLocker lock(device->m_RequestLock);
@@ -268,8 +269,9 @@ void Reader::IoSmartCardGetAttribute(IWDFIoRequest* pRequest,SIZE_T inBufSize,SI
if (rpcType==0) { if (rpcType==0) {
PipeReader *pipe=(PipeReader *)this; PipeReader *pipe=(PipeReader *)this;
OutputDebugString(L"[VivoKeySmartReader][GATT]EVENT_PIPE_NAME"); OutputDebugString(L"[VivoKeySmartReader][GATT]EVENT_PIPE_NAME");
sprintf(temp,"%S",pipe->pipeEventName); sprintf(temp,"%.*S",(int)sizeof(temp),pipe->pipeEventName);
setString(device,pRequest,(char*)temp,(int)outBufSize); temp[sizeof(temp)-1] = '\0';
setString(device,pRequest,(char*)temp,outBufSize);
} }
else { else {
SectionLocker lock(device->m_RequestLock); SectionLocker lock(device->m_RequestLock);
@@ -320,28 +322,30 @@ void Reader::IoSmartCardGetAttribute(IWDFIoRequest* pRequest,SIZE_T inBufSize,SI
return; return;
case SCARD_ATTR_VENDOR_NAME: case SCARD_ATTR_VENDOR_NAME:
OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_VENDOR_NAME"); OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_VENDOR_NAME");
setString(device,pRequest,vendorName,(int)outBufSize); setString(device,pRequest,vendorName,outBufSize);
return; return;
case SCARD_ATTR_VENDOR_IFD_TYPE: case SCARD_ATTR_VENDOR_IFD_TYPE:
OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_VENDOR_IFD_TYPE"); OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_VENDOR_IFD_TYPE");
setString(device,pRequest,vendorIfdType,(int)outBufSize); setString(device,pRequest,vendorIfdType,outBufSize);
return; return;
case SCARD_ATTR_DEVICE_UNIT: case SCARD_ATTR_DEVICE_UNIT:
OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_DEVICE_UNIT"); OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_DEVICE_UNIT");
setInt(device,pRequest,deviceUnit); setInt(device,pRequest,deviceUnit);
return; return;
case SCARD_ATTR_ATR_STRING: case SCARD_ATTR_ATR_STRING:
OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_ATR_STRING");
BYTE ATR[100];
DWORD ATRsize;
if (!QueryATR(ATR,&ATRsize))
{ {
SectionLocker lock(device->m_RequestLock); OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_ATR_STRING");
pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0); BYTE ATR[100];
DWORD ATRsize=sizeof(ATR);
if (!QueryATR(ATR,&ATRsize))
{
SectionLocker lock(device->m_RequestLock);
pRequest->CompleteWithInformation(STATUS_NO_MEDIA, 0);
return;
}
setBuffer(device,pRequest,ATR,ATRsize);
return; return;
} }
setBuffer(device,pRequest,ATR,ATRsize);
return;
case SCARD_ATTR_CURRENT_PROTOCOL_TYPE: case SCARD_ATTR_CURRENT_PROTOCOL_TYPE:
OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_CURRENT_PROTOCOL_TYPE"); OutputDebugString(L"[VivoKeySmartReader][GATT]SCARD_ATTR_CURRENT_PROTOCOL_TYPE");
setInt(device,pRequest,protocol); // T=0 or T=1 setInt(device,pRequest,protocol); // T=0 or T=1
@@ -377,7 +381,7 @@ bool Reader::QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset) {
bool Reader::initProtocols() { bool Reader::initProtocols() {
// ask ATR to determine available protocols // ask ATR to determine available protocols
BYTE ATR[100]; BYTE ATR[100];
DWORD ATRsize=100; DWORD ATRsize=sizeof(ATR);
availableProtocol=0; availableProtocol=0;
if (QueryATR(ATR,&ATRsize,true)) if (QueryATR(ATR,&ATRsize,true))
{ {

View File

@@ -107,6 +107,7 @@ bool TcpIpReader::QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset) {
} }
if (size==0) if (size==0)
return false; return false;
size=min(size,*ATRsize);
if ((read=recv(AcceptSocket,(char*)ATR,size,MSG_WAITALL))<=0) { if ((read=recv(AcceptSocket,(char*)ATR,size,MSG_WAITALL))<=0) {
::shutdown(AcceptSocket,SD_BOTH); ::shutdown(AcceptSocket,SD_BOTH);
AcceptSocket=NULL; AcceptSocket=NULL;

View File

@@ -70,6 +70,7 @@ bool VpcdReader::QueryATR(BYTE *ATR,DWORD *ATRsize,bool reset) {
if (atr_len > 0) { if (atr_len > 0) {
/* TODO do length checking on length of ATR when ATRsize is /* TODO do length checking on length of ATR when ATRsize is
* correctly initialized by Reader.cpp */ * correctly initialized by Reader.cpp */
atr_len = min(atr_len, *ATRsize);
memcpy(ATR, atr, atr_len); memcpy(ATR, atr, atr_len);
*ATRsize = atr_len; *ATRsize = atr_len;
free(atr); free(atr);

View File

@@ -1,7 +1,7 @@
#include "memory.h" #include "memory.h"
#include "SectionLocker.h" #include "SectionLocker.h"
bool getBuffer(IWDFIoRequest* pRequest,void **buffer,int *bufferLen) { bool getBuffer(IWDFIoRequest* pRequest,void **buffer,SIZE_T *bufferLen) {
IWDFMemory *inmem=NULL; IWDFMemory *inmem=NULL;
pRequest->GetInputMemory(&inmem); pRequest->GetInputMemory(&inmem);
if (inmem==NULL) { if (inmem==NULL) {
@@ -20,13 +20,13 @@ bool getBuffer(IWDFIoRequest* pRequest,void **buffer,int *bufferLen) {
memcpy(out,data,size); memcpy(out,data,size);
(*buffer)=out; (*buffer)=out;
} }
(*bufferLen)=(int)size; (*bufferLen)=size;
inmem->Release(); inmem->Release();
return true; return true;
} }
} }
void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,int inSize) { void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,SIZE_T inSize) {
IWDFMemory *outmem=NULL; IWDFMemory *outmem=NULL;
pRequest->GetOutputMemory (&outmem); pRequest->GetOutputMemory (&outmem);
if (outmem==NULL) { if (outmem==NULL) {
@@ -42,7 +42,7 @@ void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,int inSize
} }
} }
void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,int outSize) { void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,SIZE_T outSize) {
IWDFMemory *outmem=NULL; IWDFMemory *outmem=NULL;
pRequest->GetOutputMemory (&outmem); pRequest->GetOutputMemory (&outmem);
if (outmem==NULL) { if (outmem==NULL) {
@@ -52,7 +52,7 @@ void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,int outSiz
} }
else { else {
SectionLocker lock(device->m_RequestLock); SectionLocker lock(device->m_RequestLock);
int size=min(outSize,(int)strlen(result)+1); SIZE_T size=min(outSize,strlen(result)+1);
outmem->CopyFromBuffer(0,result,size); outmem->CopyFromBuffer(0,result,size);
outmem->Release(); outmem->Release();
pRequest->CompleteWithInformation(0,(SIZE_T)size); pRequest->CompleteWithInformation(0,(SIZE_T)size);
@@ -84,6 +84,10 @@ DWORD getInt(IWDFIoRequest* pRequest) {
else { else {
SIZE_T size; SIZE_T size;
void *data=inmem->GetDataBuffer(&size); void *data=inmem->GetDataBuffer(&size);
if (size<sizeof(DWORD)) {
OutputDebugString(L"Invalid input");
return 0xffffffff;
}
DWORD d=*(DWORD *)data; DWORD d=*(DWORD *)data;
inmem->Release(); inmem->Release();
return d; return d;

View File

@@ -3,8 +3,8 @@
#include "device.h" #include "device.h"
bool getBuffer(IWDFIoRequest* pRequest,void **buffer,int *bufferLen); bool getBuffer(IWDFIoRequest* pRequest,void **buffer,SIZE_T *bufferLen);
void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,int outSize); void setString(CMyDevice *device,IWDFIoRequest* pRequest,char *result,SIZE_T outSize);
void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,int inSize); void setBuffer(CMyDevice *device,IWDFIoRequest* pRequest,void *result,SIZE_T inSize);
void setInt(CMyDevice *device,IWDFIoRequest* pRequest,DWORD result); void setInt(CMyDevice *device,IWDFIoRequest* pRequest,DWORD result);
DWORD getInt(IWDFIoRequest* pRequest); DWORD getInt(IWDFIoRequest* pRequest);