diff --git a/0001-T0ProcACK-fix-a-potential-problem.patch b/0001-T0ProcACK-fix-a-potential-problem.patch new file mode 100644 index 0000000000000000000000000000000000000000..b8f1725df93784e793decfb7b2394191862bac9f --- /dev/null +++ b/0001-T0ProcACK-fix-a-potential-problem.patch @@ -0,0 +1,46 @@ +From 1e1166661ef5c6776189aeed09b39f1a91e107e3 Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/0002-CmdGetSlotStatus-fix-potential-read-of-uninitialized.patch b/0002-CmdGetSlotStatus-fix-potential-read-of-uninitialized.patch new file mode 100644 index 0000000000000000000000000000000000000000..88bf7def0194d3ac0ca770a85fa849d4a3f9300c --- /dev/null +++ b/0002-CmdGetSlotStatus-fix-potential-read-of-uninitialized.patch @@ -0,0 +1,42 @@ +From 09a6323de16c720e68abea8deb78b864942bd3da Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/0003-ReadUSB-fix-potential-read-of-uninitialized-buffer.patch b/0003-ReadUSB-fix-potential-read-of-uninitialized-buffer.patch new file mode 100644 index 0000000000000000000000000000000000000000..3cab0c63e9c14197ba1ad9e78e309516536c18fa --- /dev/null +++ b/0003-ReadUSB-fix-potential-read-of-uninitialized-buffer.patch @@ -0,0 +1,37 @@ +From 5376fa1d7a8f207a075602c81e6e5e993abe2bd3 Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/0004-IFDHSetProtocolParameters-handle-ATR_GetConvention-e.patch b/0004-IFDHSetProtocolParameters-handle-ATR_GetConvention-e.patch new file mode 100644 index 0000000000000000000000000000000000000000..5e84c8ce4e7efbcadfe2dc99778743e3cb897622 --- /dev/null +++ b/0004-IFDHSetProtocolParameters-handle-ATR_GetConvention-e.patch @@ -0,0 +1,41 @@ +From 2c1ce06df39f17821e4b1891c09e8399bf10ad7f Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/0005-PPS_Match-fix-potential-read-of-uninitialized-buffer.patch b/0005-PPS_Match-fix-potential-read-of-uninitialized-buffer.patch new file mode 100644 index 0000000000000000000000000000000000000000..78399ba49f83750e8c15f6a6925314868b661b09 --- /dev/null +++ b/0005-PPS_Match-fix-potential-read-of-uninitialized-buffer.patch @@ -0,0 +1,39 @@ +From 94f3619b2efbb852c4fc0cb42b20755bc7bf380b Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/0006-dw2i-fix-potential-integer-overflow.patch b/0006-dw2i-fix-potential-integer-overflow.patch new file mode 100644 index 0000000000000000000000000000000000000000..3893f070d739561d93ab2fdd5630415a23c2d278 --- /dev/null +++ b/0006-dw2i-fix-potential-integer-overflow.patch @@ -0,0 +1,42 @@ +From fde8bd2ece2cc4422c326fc30f399e39965481d8 Mon Sep 17 00:00:00 2001 +From: Ludovic Rousseau +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 + diff --git a/ccid.spec b/ccid.spec index 2bf4026c861506668fe6b85a620401ec2c1e2c2a..107da4fea21a603f7709686a2646c6ac3c1bab99 100644 --- a/ccid.spec +++ b/ccid.spec @@ -2,14 +2,21 @@ Name: ccid Version: 1.4.33 -Release: 1 +Release: 3 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 -BuildRequires: perl-interpreter perl-Getopt-Long libusb1-devel gnupg2 gcc git +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 BuildRequires: pcsc-lite-devel >= 1.8.9 Requires(post): systemd Requires(postun): systemd @@ -24,7 +31,7 @@ Interface Devices) driver and ICCD (Integrated Circuit(s) Card Devices).See the USB CCID and ICCD specifications from the USB working group. %prep -%autosetup -n ccid-%{version} -p1 -S git +%autosetup -n ccid-%{version} -p1 %build %configure --enable-twinserial @@ -48,6 +55,12 @@ cp -p src/openct/LICENSE LICENSE.openct %config(noreplace) %{_sysconfdir}/reader.conf.d/libccidtwin %changelog +* Fri Jul 30 2021 chenyanpanHW - 1.4.33-3 +- DESC: delete -S git from %autosetup, and delete BuildRequires git + +* Fri Oct 30 2020 Zhiqiang Liu - 1.4.33-2 +- backport some patches to fix some problems. + * Wed Jul 29 2020 yanglongkang - 1.4.33-1 - update to 1.4.33 version