pcsc-lite: backport some patches to solve some upstream problems

backport some patches to solve some upstream problems

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
This commit is contained in:
Zhiqiang Liu 2020-10-29 22:09:48 +08:00
parent 400fdaf23b
commit 14048123a2
5 changed files with 230 additions and 1 deletions

View File

@ -0,0 +1,78 @@
From 36bc9446b40fa3c6ac12312b934f4d7131659087 Mon Sep 17 00:00:00 2001
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
Date: Wed, 5 Aug 2020 17:59:41 +0200
Subject: [PATCH 01/13] Do not (possibly) lock a reader if allocating hCard
fails
In case of SCardConnect() the reader may be locked in
SCARD_SHARE_EXCLUSIVE mode if internal SCardConnect() works but
MSGAddHandle() fails because the list of handle is full.
You need to start pcscd with "--max-card-handle-per-reader n" with
n > 200 or the 200 limit (default value) will be hit in internal
SCardConnect() and MSGAddHandle() would not be called.
Thanks to Maksim Ivanov for the bug report
"[Pcsclite-muscle] SCardConnect behavior with invalid contexts"
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-July/001095.html
" Hello,
I believe that there's a potential problem with the SCardConnect
implementation that it doesn't check the received SCARDCONTEXT
*before* executing the command. This might result in an unexpected
state, where the SCardConnect() caller receives an error code
meanwhile the connection to the card is actually established (which,
for example, might be an exclusive connection that prevents anyone
else from connecting to the card).
In detail, the ContextThread() function in winscard_svc.c, when
receiving the SCARD_CONNECT command, calls first SCardConnect() from
winscard.c, and then MSGAddHandle(). The former ignores SCARDCONTEXT
and, if possible, establishes a connection to the card. The latter
does check the SCARDCONTEXT value, but this happens after the
connection is already established, and its error is just returned to
the caller (without closing the just-opened connection).
Would it make sense to add a check of SCARDCONTEXT before calling
SCardConnect(), and/or to call SCardDisconnect() if MSGAddHandle()
fails?
Regards,
Maksim "
---
src/winscard_svc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/winscard_svc.c b/src/winscard_svc.c
index cdeac33..c0df008 100644
--- a/src/winscard_svc.c
+++ b/src/winscard_svc.c
@@ -507,9 +507,15 @@ static void * ContextThread(LPVOID newContext)
coStr.dwActiveProtocol = dwActiveProtocol;
if (coStr.rv == SCARD_S_SUCCESS)
+ {
coStr.rv = MSGAddHandle(coStr.hContext, coStr.hCard,
threadContext);
+ /* if storing the hCard fails we disconnect */
+ if (coStr.rv != SCARD_S_SUCCESS)
+ SCardDisconnect(coStr.hCard, SCARD_LEAVE_CARD);
+ }
+
WRITE_BODY(coStr);
}
break;
@@ -963,7 +969,7 @@ static LONG MSGAddHandle(SCARDCONTEXT hContext, SCARDHANDLE hCard,
if (listLength >= contextMaxCardHandles)
{
Log4(PCSC_LOG_DEBUG,
- "Too many card handles for thread context @%p: %d (max is %d)"
+ "Too many card handles for thread context @%p: %d (max is %d). "
"Restart pcscd with --max-card-handle-per-thread value",
threadContext, listLength, contextMaxCardHandles);
retval = SCARD_E_NO_MEMORY;
--
1.8.3.1

View File

@ -0,0 +1,56 @@
From 38dfe5c1f474db519e1f7e31cf714ba5d4c6cfa4 Mon Sep 17 00:00:00 2001
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
Date: Wed, 5 Aug 2020 18:57:30 +0200
Subject: [PATCH 02/13] Fix a hang in SCardTransmit()
In some special conditions it is possible to make SCardTransmit() to
hang forever in pcscd and generates a denial of service.
I was able to reproduce the problem using a sample C code.
Thanks to Maksim Ivanov for the bug report
"[Pcsclite-muscle] Potential hang in SCardTransmit"
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-July/001096.html
" Hello,
It seems that there's (at least half-hypothetical) scenario when
SCardTransmit may hang.
The combination is:
the service's |readerState| is (SCARD_PRESENT | SCARD_POWERED |
SCARD_NEGOTIABLE);
the service's |cardProtocol| is SCARD_PROTOCOL_UNDEFINED (right after
power-up);
the caller's |pioSendPci->dwProtocol| is SCARD_PROTOCOL_ANY_OLD.
In that case, the hang happens in the loop that attempts to find the
highest bit in the |cardProtocol| value; it doesn't handle the case
when the latter is zero:
https://salsa.debian.org/rousseau/PCSC/-/blob/467df10d439f6d739cd48a51f2b3dd543b1a64ce/src/winscard.c#L1583
P.S. Sorry if I misunderstood something and this case can never occur
in practice.
Regards,
Maksim "
---
src/winscard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/winscard.c b/src/winscard.c
index 9f24cd7..3b88554 100644
--- a/src/winscard.c
+++ b/src/winscard.c
@@ -1580,7 +1580,7 @@ LONG SCardTransmit(SCARDHANDLE hCard, const SCARD_IO_REQUEST *pioSendPci,
unsigned long i;
unsigned long prot = rContext->readerState->cardProtocol;
- for (i = 0 ; prot != 1 ; i++)
+ for (i = 0 ; prot != 1 && i < 16; i++)
prot >>= 1;
sSendPci.Protocol = i;
--
1.8.3.1

View File

@ -0,0 +1,52 @@
From a706455f31178ab35f07e3e6e76bd4a35d7ef3da Mon Sep 17 00:00:00 2001
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
Date: Sat, 8 Aug 2020 15:11:53 +0200
Subject: [PATCH 03/13] ATRDecodeAtr: always initialize the return values
Always set a value to availableProtocols and currentProtocol before any
return in error.
Thanks to Maksim Ivanov for the bug report
"[Pcsclite-muscle] Missing checks of ATRDecodeAtr returns"
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-July/001097.html
" Hello,
The callers of the ATRDecodeAtr() function (SCardConnect() and
SCardReconnect() in winscard.c) don't check its return value, which
might potentially cause reads of uninitialized variables
|availableProtocols| and |defaultProtocol| and unexpected side
effects.
Regards,
Maksim "
---
src/atrhandler.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/atrhandler.c b/src/atrhandler.c
index 2ebc440..1e0654d 100644
--- a/src/atrhandler.c
+++ b/src/atrhandler.c
@@ -75,15 +75,15 @@ short ATRDecodeAtr(int *availableProtocols, int *currentProtocol,
LogXxd(PCSC_LOG_DEBUG, "ATR: ", pucAtr, dwLength);
#endif
- if (dwLength < 2)
- return 0; /** @retval 0 Atr must have TS and T0 */
-
/*
* Zero out the bitmasks
*/
*availableProtocols = SCARD_PROTOCOL_UNDEFINED;
*currentProtocol = SCARD_PROTOCOL_UNDEFINED;
+ if (dwLength < 2)
+ return 0; /** @retval 0 Atr must have TS and T0 */
+
/*
* Decode the TS byte
*/
--
1.8.3.1

View File

@ -0,0 +1,36 @@
From 278b55a87a5f4b9bd86513f7d8f9ab7d66558602 Mon Sep 17 00:00:00 2001
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
Date: Sat, 8 Aug 2020 17:37:40 +0200
Subject: [PATCH 05/13] EHUnregisterClientForEvent: correctly handle
EHTryToUnregisterClientForEvent
EHTryToUnregisterClientForEvent() returns SCARD_S_SUCCESS or
SCARD_F_INTERNAL_ERROR but never a negative value.
Thanks to Valerii Zapodovnikov for the bug report
"Code cleanup"
https://salsa.debian.org/rousseau/PCSC/-/issues/19
" https://salsa.debian.org/rousseau/PCSC/-/blob/master/src/eventhandler.c#L107
rv < 0 is always false, because on line 94 there SCARD_F_INTERNAL_ERROR
is ((LONG)0x80100001 and SCARD_S_SUCCESS is ((LONG)0x00000000). "
---
src/eventhandler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/eventhandler.c b/src/eventhandler.c
index 932d30b..8d450d5 100644
--- a/src/eventhandler.c
+++ b/src/eventhandler.c
@@ -104,7 +104,7 @@ LONG EHUnregisterClientForEvent(int32_t filedes)
{
LONG rv = EHTryToUnregisterClientForEvent(filedes);
- if (rv < 0)
+ if (rv != SCARD_S_SUCCESS)
Log2(PCSC_LOG_ERROR, "Can't remove client: %d", filedes);
return rv;
--
1.8.3.1

View File

@ -1,6 +1,6 @@
Name: pcsc-lite Name: pcsc-lite
Version: 1.9.0 Version: 1.9.0
Release: 1 Release: 2
Summary: Middleware to access a smart card using SCard API (PC/SC) Summary: Middleware to access a smart card using SCard API (PC/SC)
License: BSD License: BSD
URL: https://pcsclite.apdu.fr/ URL: https://pcsclite.apdu.fr/
@ -19,6 +19,10 @@ Provides: pcsc-lite-libs%{?_isa} pcsc-lite-libs
Obsoletes: pcsc-lite-libs Obsoletes: pcsc-lite-libs
Patch0: 0000-pcsc-lite-change-to-use-python3-for-pcsc-spy.patch Patch0: 0000-pcsc-lite-change-to-use-python3-for-pcsc-spy.patch
Patch1: 0001-Do-not-possibly-lock-a-reader-if-allocating-hCard-fa.patch
Patch2: 0002-Fix-a-hang-in-SCardTransmit.patch
Patch3: 0003-ATRDecodeAtr-always-initialize-the-return-values.patch
Patch4: 0004-EHUnregisterClientForEvent-correctly-handle-EHTryToU.patch
%description %description
PC/SC Lite is a middleware to access a smart card using SCard API (PC/SC). PC/SC Lite is a middleware to access a smart card using SCard API (PC/SC).
@ -112,6 +116,9 @@ mkdir -p %{buildroot}/%{_localstatedir}/run/pcscd
%changelog %changelog
* Thu Oct 29 2020 Zhiqiang Liu <liuzhiqiang26@huawei.com> - 1.9.0-2
- backport some patches to solve some upstream problems
* Tue Jul 21 2020 jixinjie <jixinjie@huawei.com> - 1.9.0-1 * Tue Jul 21 2020 jixinjie <jixinjie@huawei.com> - 1.9.0-1
- update package to 1.9.0 - update package to 1.9.0