ccid: backport some patches to fix some potential problems.
backport some patches to fix some potential problems. Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
This commit is contained in:
Zhiqiang Liu
parent
31703e848b
commit
bb02a0e9fc
46
0001-T0ProcACK-fix-a-potential-problem.patch
Normal file
46
0001-T0ProcACK-fix-a-potential-problem.patch
Normal file
@ -0,0 +1,46 @@
|
||||
From 1e1166661ef5c6776189aeed09b39f1a91e107e3 Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 15:39:17 +0200
|
||||
Subject: [PATCH 1/6] T0ProcACK: fix a potential problem
|
||||
|
||||
" Apparently, the fuzzer found one more similar bug: T0ProcACK() can be
|
||||
called with the |proc_len| parameter equal to -1, leading to
|
||||
stack-buffer-overflow.
|
||||
|
||||
The stack trace is:
|
||||
|
||||
#1 0x56eee7 in T0ProcACK /ssd/ccid/src/fuzzer/../commands.c:1988:3
|
||||
#2 0x56d1d1 in CmdXfrBlockCHAR_T0 /ssd/ccid/src/fuzzer/../commands.c:2253:20
|
||||
#3 0x5754cc in IFDHTransmitToICC /ssd/ccid/src/fuzzer/../ifdhandler.c:1403:17
|
||||
|
||||
and the T0ProcACK() call is made from this line:
|
||||
https://salsa.debian.org/rousseau/CCID/-/blob/c122e4f38cc7d1ffdb1fc0cece49145930d4634a/src/commands.c#L2197
|
||||
|
||||
The negative |proc_len| is the result of this equation: |exp_len -
|
||||
*rcv_len|, with exp_len=2, *rcv_len=3 in the found scenario. "
|
||||
|
||||
The problem has been found by an automatic buzzer, not by a real problem
|
||||
in the field.
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
---
|
||||
src/commands.c | 3 +++
|
||||
1 file changed, 3 insertions(+)
|
||||
|
||||
diff --git a/src/commands.c b/src/commands.c
|
||||
index 07bad44..c00c2d5 100644
|
||||
--- a/src/commands.c
|
||||
+++ b/src/commands.c
|
||||
@@ -1852,6 +1852,9 @@ static RESPONSECODE T0ProcACK(unsigned int reader_index,
|
||||
|
||||
DEBUG_COMM2("Enter, is_rcv = %d", is_rcv);
|
||||
|
||||
+ if (proc_len < 0)
|
||||
+ return IFD_COMMUNICATION_ERROR;
|
||||
+
|
||||
if (is_rcv == 1)
|
||||
{ /* Receiving mode */
|
||||
unsigned int remain_len;
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
@ -0,0 +1,42 @@
|
||||
From 09a6323de16c720e68abea8deb78b864942bd3da Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 16:28:32 +0200
|
||||
Subject: [PATCH 2/6] CmdGetSlotStatus: fix potential read of uninitialized
|
||||
buffer
|
||||
|
||||
If the command SlotStatus fails then we report: card absent.
|
||||
The problem was only present for a ICCD type B reader.
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
"[Pcsclite-muscle] Insufficient checks in CCID"
|
||||
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-August/001098.html
|
||||
|
||||
" Hello,
|
||||
|
||||
The CCID free software driver is missing a few checks and graceful
|
||||
handling of some error cases:
|
||||
|
||||
4. Read of uninitialized buffer in CmdGetSlotStatus() at
|
||||
https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/commands.c#L1201
|
||||
- in case when the control transfer returned only 1 instead of 3
|
||||
bytes. "
|
||||
---
|
||||
src/commands.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/commands.c b/src/commands.c
|
||||
index c00c2d5..cbbb19a 100644
|
||||
--- a/src/commands.c
|
||||
+++ b/src/commands.c
|
||||
@@ -1182,7 +1182,7 @@ again_status:
|
||||
if (PROTOCOL_ICCD_B == ccid_descriptor->bInterfaceProtocol)
|
||||
{
|
||||
int r;
|
||||
- unsigned char buffer_tmp[3];
|
||||
+ unsigned char buffer_tmp[3] = {0, 2, 0};
|
||||
|
||||
/* SlotStatus */
|
||||
r = ControlUSB(reader_index, 0xA1, 0x81, 0, buffer_tmp,
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
@ -0,0 +1,37 @@
|
||||
From 5376fa1d7a8f207a075602c81e6e5e993abe2bd3 Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 16:34:21 +0200
|
||||
Subject: [PATCH 3/6] ReadUSB: fix potential read of uninitialized buffer
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
"[Pcsclite-muscle] Insufficient checks in CCID"
|
||||
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-August/001098.html
|
||||
|
||||
" Hello,
|
||||
|
||||
The CCID free software driver is missing a few checks and graceful
|
||||
handling of some error cases:
|
||||
|
||||
5. Read of uninitialized buffer in ReadUSB() at
|
||||
https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/ccid_usb.c#L912
|
||||
. (Because of the wrong ">=" size check - it should be a strict ">".) "
|
||||
---
|
||||
src/ccid_usb.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/ccid_usb.c b/src/ccid_usb.c
|
||||
index 48fdc5c..85fce4a 100644
|
||||
--- a/src/ccid_usb.c
|
||||
+++ b/src/ccid_usb.c
|
||||
@@ -908,7 +908,7 @@ read_again:
|
||||
DEBUG_XXD(debug_header, buffer, *length);
|
||||
|
||||
#define BSEQ_OFFSET 6
|
||||
- if ((*length >= BSEQ_OFFSET)
|
||||
+ if ((*length >= BSEQ_OFFSET +1)
|
||||
&& (buffer[BSEQ_OFFSET] < *ccid_descriptor->pbSeq -1))
|
||||
{
|
||||
duplicate_frame++;
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
@ -0,0 +1,41 @@
|
||||
From 2c1ce06df39f17821e4b1891c09e8399bf10ad7f Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 16:39:04 +0200
|
||||
Subject: [PATCH 4/6] IFDHSetProtocolParameters: handle ATR_GetConvention()
|
||||
error
|
||||
|
||||
If the ATR is invalid (i.e. does not start with 0x3B or 0x3F) then we
|
||||
return an error instead of using an unitialized value.
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
"[Pcsclite-muscle] Insufficient checks in CCID"
|
||||
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-August/001098.html
|
||||
|
||||
" Hello,
|
||||
|
||||
The CCID free software driver is missing a few checks and graceful
|
||||
handling of some error cases:
|
||||
|
||||
6. Read of uninitialized |convention| in IFDHSetProtocolParameters() -
|
||||
in case ATR_GetConvention() returned a failure on a malformed ATR. "
|
||||
---
|
||||
src/ifdhandler.c | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/ifdhandler.c b/src/ifdhandler.c
|
||||
index 1d2281e..0deb9d8 100644
|
||||
--- a/src/ifdhandler.c
|
||||
+++ b/src/ifdhandler.c
|
||||
@@ -943,7 +943,8 @@ EXTERNAL RESPONSECODE IFDHSetProtocolParameters(DWORD Lun, DWORD Protocol,
|
||||
}
|
||||
|
||||
/* Now we must set the reader parameters */
|
||||
- (void)ATR_GetConvention(&atr, &convention);
|
||||
+ if (ATR_MALFORMED == ATR_GetConvention(&atr, &convention))
|
||||
+ return IFD_COMMUNICATION_ERROR;
|
||||
|
||||
/* specific mode and implicit parameters? (b5 of TA2) */
|
||||
if (atr.ib[1][ATR_INTERFACE_BYTE_TA].present
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
@ -0,0 +1,39 @@
|
||||
From 94f3619b2efbb852c4fc0cb42b20755bc7bf380b Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 16:45:17 +0200
|
||||
Subject: [PATCH 5/6] PPS_Match: fix potential read of uninitialized buffer
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
"[Pcsclite-muscle] Insufficient checks in CCID"
|
||||
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-August/001098.html
|
||||
|
||||
" Hello,
|
||||
|
||||
The CCID free software driver is missing a few checks and graceful
|
||||
handling of some error cases:
|
||||
|
||||
7. Read of uninitialized buffer in PPS_Match() at
|
||||
https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/towitoko/pps.c#L101
|
||||
- in case |len_confirm| is unexpectedly small. "
|
||||
---
|
||||
src/towitoko/pps.c | 4 +++-
|
||||
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/towitoko/pps.c b/src/towitoko/pps.c
|
||||
index d3b9bda..82b5915 100644
|
||||
--- a/src/towitoko/pps.c
|
||||
+++ b/src/towitoko/pps.c
|
||||
@@ -98,7 +98,9 @@ PPS_Match (BYTE * request, unsigned len_request, BYTE * confirm, unsigned len_co
|
||||
return FALSE;
|
||||
|
||||
/* See if the card specifies other than default FI and D */
|
||||
- if ((PPS_HAS_PPS1 (confirm)) && (confirm[2] != request[2]))
|
||||
+ if ((PPS_HAS_PPS1 (confirm))
|
||||
+ && (len_confirm > 2)
|
||||
+ && (confirm[2] != request[2]))
|
||||
return FALSE;
|
||||
|
||||
return TRUE;
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
42
0006-dw2i-fix-potential-integer-overflow.patch
Normal file
42
0006-dw2i-fix-potential-integer-overflow.patch
Normal file
@ -0,0 +1,42 @@
|
||||
From fde8bd2ece2cc4422c326fc30f399e39965481d8 Mon Sep 17 00:00:00 2001
|
||||
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
|
||||
Date: Sat, 8 Aug 2020 17:23:40 +0200
|
||||
Subject: [PATCH 6/6] dw2i: fix potential integer overflow
|
||||
|
||||
The maximum values manipulated by dw2i() should be far less than 64 KB.
|
||||
So the problem should never happen unless you use a malicious reader.
|
||||
|
||||
Thanks to Maksim Ivanov for the bug report
|
||||
"[Pcsclite-muscle] Instances of Undefined behavior in CCID"
|
||||
http://lists.infradead.org/pipermail/pcsclite-muscle/2020-August/001102.html
|
||||
|
||||
" Hello,
|
||||
|
||||
I found a couple of issues using the Clang's UBSan
|
||||
(https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) in the
|
||||
CCID driver implementation:
|
||||
|
||||
1. The dw2i() macro doesn't cast the shifted operands to |unsigned
|
||||
int|, which means that the compiler will use |int| for those
|
||||
intermediate expressions - but that leads to hitting Undefined
|
||||
Behavior if the values overflow the (signed) int. "
|
||||
---
|
||||
src/ccid.h | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/ccid.h b/src/ccid.h
|
||||
index c0f126c..1ad6b0d 100644
|
||||
--- a/src/ccid.h
|
||||
+++ b/src/ccid.h
|
||||
@@ -272,7 +272,7 @@ void ccid_error(int log_level, int error, const char *file, int line,
|
||||
_ccid_descriptor *get_ccid_descriptor(unsigned int reader_index);
|
||||
|
||||
/* convert a 4 byte integer in USB format into an int */
|
||||
-#define dw2i(a, x) (unsigned int)((((((a[x+3] << 8) + a[x+2]) << 8) + a[x+1]) << 8) + a[x])
|
||||
+#define dw2i(a, x) (unsigned int)(((((((unsigned int)a[x+3] << 8) + (unsigned int)a[x+2]) << 8) + (unsigned int)a[x+1]) << 8) + (unsigned int)a[x])
|
||||
|
||||
/* all the data rates specified by ISO 7816-3 Fi/Di tables */
|
||||
#define ISO_DATA_RATES 10753, 14337, 15625, 17204, \
|
||||
--
|
||||
1.8.3.1
|
||||
|
||||
12
ccid.spec
12
ccid.spec
@ -2,13 +2,20 @@
|
||||
|
||||
Name: ccid
|
||||
Version: 1.4.33
|
||||
Release: 1
|
||||
Release: 2
|
||||
Summary: Provide a generic USB CCID driver and ICCD
|
||||
License: LGPLv2+
|
||||
URL: https://ccid.apdu.fr/files/
|
||||
|
||||
Source0: https://ccid.apdu.fr/files/ccid-%{version}.tar.bz2
|
||||
|
||||
Patch1: 0001-T0ProcACK-fix-a-potential-problem.patch
|
||||
Patch2: 0002-CmdGetSlotStatus-fix-potential-read-of-uninitialized.patch
|
||||
Patch3: 0003-ReadUSB-fix-potential-read-of-uninitialized-buffer.patch
|
||||
Patch4: 0004-IFDHSetProtocolParameters-handle-ATR_GetConvention-e.patch
|
||||
Patch5: 0005-PPS_Match-fix-potential-read-of-uninitialized-buffer.patch
|
||||
Patch6: 0006-dw2i-fix-potential-integer-overflow.patch
|
||||
|
||||
BuildRequires: perl-interpreter perl-Getopt-Long libusb1-devel gnupg2 gcc git
|
||||
BuildRequires: pcsc-lite-devel >= 1.8.9
|
||||
Requires(post): systemd
|
||||
@ -48,6 +55,9 @@ cp -p src/openct/LICENSE LICENSE.openct
|
||||
%config(noreplace) %{_sysconfdir}/reader.conf.d/libccidtwin
|
||||
|
||||
%changelog
|
||||
* Fri Oct 30 2020 Zhiqiang Liu <liuzhiqiang26@huawei.com> - 1.4.33-2
|
||||
- backport some patches to fix some problems.
|
||||
|
||||
* Tue Jul 21 2020 Zhiqiang Liu <liuzhiqiang26@huawei.com> - 1.4.33-1
|
||||
- update to 1.4.33 version
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user