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