From 527e3ae0da5c6147197941b43f1601207bb1186d Mon Sep 17 00:00:00 2001 From: Jan Grulich Date: Thu, 4 Jan 2024 09:05:21 +0100 Subject: [PATCH] Fix incorrect integer overflow check in HTTP2 implementation Resolves: RHEL-20239 --- 0001-CVE-2023-51714-qtbase-5.15.patch | 38 +++++++++++++++++ 0002-CVE-2023-51714-qtbase-5.15.patch | 59 +++++++++++++++++++++++++++ qt5-qtbase.spec | 10 ++++- 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 0001-CVE-2023-51714-qtbase-5.15.patch create mode 100644 0002-CVE-2023-51714-qtbase-5.15.patch diff --git a/0001-CVE-2023-51714-qtbase-5.15.patch b/0001-CVE-2023-51714-qtbase-5.15.patch new file mode 100644 index 0000000..771b8c0 --- /dev/null +++ b/0001-CVE-2023-51714-qtbase-5.15.patch @@ -0,0 +1,38 @@ +From ea63c28efc1d2ecb467b83a34923d12462efa96f Mon Sep 17 00:00:00 2001 +From: Marc Mutz +Date: Tue, 12 Dec 2023 20:51:56 +0100 +Subject: [PATCH] HPack: fix a Yoda Condition + +Putting the variable on the LHS of a relational operation makes the +expression easier to read. In this case, we find that the whole +expression is nonsensical as an overflow protection, because if +name.size() + value.size() overflows, the result will exactly _not_ +be > max() - 32, because UB will have happened. + +To be fixed in a follow-up commit. + +As a drive-by, add parentheses around the RHS. + +Change-Id: I35ce598884c37c51b74756b3bd2734b9aad63c09 +Reviewed-by: Allan Sandfeld Jensen +(cherry picked from commit 658607a34ead214fbacbc2cca44915655c318ea9) +Reviewed-by: Qt Cherry-pick Bot +(cherry picked from commit 4f7efd41740107f90960116700e3134f5e433867) +(cherry picked from commit 13c16b756900fe524f6d9534e8a07aa003c05e0c) +(cherry picked from commit 1d4788a39668fb2dc5912a8d9c4272dc40e99f92) +(cherry picked from commit 87de75b5cc946d196decaa6aef4792a6cac0b6db) +--- + +diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp +index 834214f..ab166a6 100644 +--- a/src/network/access/http2/hpacktable.cpp ++++ b/src/network/access/http2/hpacktable.cpp +@@ -63,7 +63,7 @@ + // 32 octets of overhead." + + const unsigned sum = unsigned(name.size() + value.size()); +- if (std::numeric_limits::max() - 32 < sum) ++ if (sum > (std::numeric_limits::max() - 32)) + return HeaderSize(); + return HeaderSize(true, quint32(sum + 32)); + } diff --git a/0002-CVE-2023-51714-qtbase-5.15.patch b/0002-CVE-2023-51714-qtbase-5.15.patch new file mode 100644 index 0000000..b002d59 --- /dev/null +++ b/0002-CVE-2023-51714-qtbase-5.15.patch @@ -0,0 +1,59 @@ +From 23c3fc483e8b6e21012a61f0bea884446f727776 Mon Sep 17 00:00:00 2001 +From: Marc Mutz +Date: Tue, 12 Dec 2023 22:08:07 +0100 +Subject: [PATCH] HPack: fix incorrect integer overflow check + +This code never worked: + +For the comparison with max() - 32 to trigger, on 32-bit platforms (or +Qt 5) signed interger overflow would have had to happen in the +addition of the two sizes. The compiler can therefore remove the +overflow check as dead code. + +On Qt 6 and 64-bit platforms, the signed integer addition would be +very unlikely to overflow, but the following truncation to uint32 +would yield the correct result only in a narrow 32-value window just +below UINT_MAX, if even that. + +Fix by using the proper tool, qAddOverflow. + +Manual conflict resolutions: + - qAddOverflow doesn't exist in Qt 5, use private add_overflow + predecessor API instead + +Change-Id: I7599f2e75ff7f488077b0c60b81022591005661c +Reviewed-by: Allan Sandfeld Jensen +(cherry picked from commit ee5da1f2eaf8932aeca02ffea6e4c618585e29e3) +Reviewed-by: Qt Cherry-pick Bot +(cherry picked from commit debeb8878da2dc706ead04b6072ecbe7e5313860) +Reviewed-by: Thiago Macieira +Reviewed-by: Marc Mutz +(cherry picked from commit 811b9eef6d08d929af8708adbf2a5effb0eb62d7) +(cherry picked from commit f931facd077ce945f1e42eaa3bead208822d3e00) +(cherry picked from commit 9ef4ca5ecfed771dab890856130e93ef5ceabef5) +Reviewed-by: MÃ¥rten Nordheim +--- + +diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp +index ab166a6..de91fc0 100644 +--- a/src/network/access/http2/hpacktable.cpp ++++ b/src/network/access/http2/hpacktable.cpp +@@ -40,6 +40,7 @@ + #include "hpacktable_p.h" + + #include ++#include + + #include + #include +@@ -62,7 +63,9 @@ + // for counting the number of references to the name and value would have + // 32 octets of overhead." + +- const unsigned sum = unsigned(name.size() + value.size()); ++ size_t sum; ++ if (add_overflow(size_t(name.size()), size_t(value.size()), &sum)) ++ return HeaderSize(); + if (sum > (std::numeric_limits::max() - 32)) + return HeaderSize(); + return HeaderSize(true, quint32(sum + 32)); diff --git a/qt5-qtbase.spec b/qt5-qtbase.spec index 8233ce2..11224cc 100644 --- a/qt5-qtbase.spec +++ b/qt5-qtbase.spec @@ -57,7 +57,7 @@ BuildRequires: pkgconfig(libsystemd) Name: qt5-qtbase Summary: Qt5 - QtBase components Version: 5.15.9 -Release: 7%{?dist} +Release: 8%{?dist} # See LGPL_EXCEPTIONS.txt, for exception details @@ -149,6 +149,8 @@ Patch112: CVE-2023-33285-qtbase-5.15.patch Patch113: CVE-2023-34410-qtbase-5.15.patch Patch114: CVE-2023-37369-qtbase-5.15.patch Patch115: CVE-2023-38197-qtbase-5.15.patch +Patch116: 0001-CVE-2023-51714-qtbase-5.15.patch +Patch117: 0002-CVE-2023-51714-qtbase-5.15.patch # gating related patches Patch200: qtbase-disable-tests-not-working-in-gating.patch @@ -438,6 +440,8 @@ Qt5 libraries used for drawing widgets and OpenGL items. %patch -P113 -p1 %patch -P114 -p1 %patch -P115 -p1 +%patch -P116 -p1 +%patch -P117 -p1 ## gating related patches %patch -P200 -p1 -b .disable-tests-not-working-in-gating @@ -1138,6 +1142,10 @@ fi %changelog +* Thu Jan 04 2024 Jan Grulich - 5.15.9-8 +- Fix incorrect integer overflow check in HTTP2 implementation + Resolves: RHEL-20239 + * Fri Jul 21 2023 Jan Grulich - 5.15.9-7 - Fix infinite loops in QXmlStreamReader (CVE-2023-38197) Resolves: bz#2222771