qt6-base: fix CVE-2025-5455

This commit is contained in:
Đoàn Trần Công Danh 2025-06-10 15:46:16 +07:00
parent aef4ad8607
commit 0059b10567
4 changed files with 312 additions and 1 deletions

View file

@ -0,0 +1,230 @@
From d29b3721988d64fdd10050918e376ae9fad8117f Mon Sep 17 00:00:00 2001
From: Shawn Rutledge <shawn.rutledge@qt.io>
Date: Thu, 27 Mar 2025 15:17:21 +0100
Subject: [PATCH] QTextMarkdownImporter: Fix heap-buffer-overflow
After finding the end marker `---`, the code expected more characters
beyond: typically at least a trailing newline. But QStringView::sliced()
crashes if asked for a substring that starts at or beyond the end.
Now it's restructured into a separate splitFrontMatter() function, and
we're stricter, tolerating only `---\n` or `---\r\n` as marker lines.
So the code is easier to prove correct, and we don't need to check
characters between the end of the marker and the end of the line
(to allow inadvertent whitespace, for example). If the markers are
not valid, the Markdown parser will see them as thematic breaks,
as it would have done if we were not extracting the Front Matter
beforehand.
Amends e10c9b5c0f8f194a79ce12dcf9b6b5cb19976942 and
bffddc6a993c4b6b64922e8d327bdf32e0d4975a
Credit to OSS-Fuzz which found this as issue 42533775.
[ChangeLog][QtGui][Text] Fixed a heap buffer overflow in
QTextMarkdownImporter. The first marker for Front Matter
must begin at the first character of a Markdown document,
and both markers must be exactly ---\n or ---\r\n.
Done-with: Marc Mutz <marc.mutz@qt.io>
Fixes: QTBUG-135284
Change-Id: I66412d21ecc0c4eabde443d70865ed2abad86d89
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
(cherry picked from commit 25986746947798e1a22d0830d3bcb11a55fcd3ae)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit eced22d7250fc7ba4dbafa1694bf149c2259d9ea)
(cherry picked from commit 9e59a924a04606c386b970ee6c9c7819cdd7ae1a)
---
diff --git a/src/gui/text/qtextmarkdownimporter.cpp b/src/gui/text/qtextmarkdownimporter.cpp
index 5f7ef22..137a0bd 100644
--- a/src/gui/text/qtextmarkdownimporter.cpp
+++ b/src/gui/text/qtextmarkdownimporter.cpp
@@ -28,7 +28,8 @@
static const QChar qtmi_Newline = u'\n';
static const QChar qtmi_Space = u' ';
-static constexpr auto markerString() noexcept { return "---"_L1; }
+static constexpr auto lfMarkerString() noexcept { return "---\n"_L1; }
+static constexpr auto crlfMarkerString() noexcept { return "---r\n"_L1; }
// TODO maybe eliminate the margins after all views recognize BlockQuoteLevel, CSS can format it, etc.
static const int qtmi_BlockQuoteIndent =
@@ -120,6 +121,47 @@
{
}
+/*! \internal
+ Split any Front Matter from the Markdown document \a md.
+ Returns a pair of QStringViews: if \a md begins with qualifying Front Matter
+ (according to the specification at https://jekyllrb.com/docs/front-matter/ ),
+ put it into the \c frontMatter view, omitting both markers; and put the remaining
+ Markdown into \c rest. If no Front Matter is found, return all of \a md in \c rest.
+*/
+static auto splitFrontMatter(QStringView md)
+{
+ struct R {
+ QStringView frontMatter, rest;
+ explicit operator bool() const noexcept { return !frontMatter.isEmpty(); }
+ };
+
+ const auto NotFound = R{{}, md};
+
+ /* Front Matter must start with '---\n' or '---\r\n' on the very first line,
+ and Front Matter must end with another such line.
+ If that is not the case, we return NotFound: then the whole document is
+ to be passed on to the Markdown parser, in which '---\n' is interpreted
+ as a "thematic break" (like <hr/> in HTML). */
+ QLatin1StringView marker;
+ if (md.startsWith(lfMarkerString()))
+ marker = lfMarkerString();
+ else if (md.startsWith(crlfMarkerString()))
+ marker = crlfMarkerString();
+ else
+ return NotFound;
+
+ const auto frontMatterStart = marker.size();
+ const auto endMarkerPos = md.indexOf(marker, frontMatterStart);
+
+ if (endMarkerPos < 0 || md[endMarkerPos - 1] != QChar::LineFeed)
+ return NotFound;
+
+ Q_ASSERT(frontMatterStart < md.size());
+ Q_ASSERT(endMarkerPos < md.size());
+ const auto frontMatter = md.sliced(frontMatterStart, endMarkerPos - frontMatterStart);
+ return R{frontMatter, md.sliced(endMarkerPos + marker.size())};
+}
+
void QTextMarkdownImporter::import(const QString &markdown)
{
MD_PARSER callbacks = {
@@ -144,21 +186,14 @@
qCDebug(lcMD) << "default font" << defaultFont << "mono font" << m_monoFont;
QStringView md = markdown;
- if (m_features.testFlag(QTextMarkdownImporter::FeatureFrontMatter) && md.startsWith(markerString())) {
- qsizetype endMarkerPos = md.indexOf(markerString(), markerString().size() + 1);
- if (endMarkerPos > 4) {
- qsizetype firstLinePos = 4; // first line of yaml
- while (md.at(firstLinePos) == '\n'_L1 || md.at(firstLinePos) == '\r'_L1)
- ++firstLinePos;
- auto frontMatter = md.sliced(firstLinePos, endMarkerPos - firstLinePos);
- firstLinePos = endMarkerPos + 4; // first line of markdown after yaml
- while (md.size() > firstLinePos && (md.at(firstLinePos) == '\n'_L1 || md.at(firstLinePos) == '\r'_L1))
- ++firstLinePos;
- md = md.sliced(firstLinePos);
- doc->setMetaInformation(QTextDocument::FrontMatter, frontMatter.toString());
- qCDebug(lcMD) << "extracted FrontMatter: size" << frontMatter.size();
+ if (m_features.testFlag(QTextMarkdownImporter::FeatureFrontMatter)) {
+ if (const auto split = splitFrontMatter(md)) {
+ doc->setMetaInformation(QTextDocument::FrontMatter, split.frontMatter.toString());
+ qCDebug(lcMD) << "extracted FrontMatter: size" << split.frontMatter.size();
+ md = split.rest;
}
}
+
const auto mdUtf8 = md.toUtf8();
m_cursor.beginEditBlock();
md_parse(mdUtf8.constData(), MD_SIZE(mdUtf8.size()), &callbacks, this);
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed1.md b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed1.md
new file mode 100644
index 0000000..8923d75
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed1.md
@@ -0,0 +1,3 @@
+---
+name: "Pluto"---
+Pluto may not be a planet. And this document does not contain Front Matter.
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed2.md b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed2.md
new file mode 100644
index 0000000..1c03291
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed2.md
@@ -0,0 +1,5 @@
+---
+name: "Sloppy"
+---
+This document has trailing whitespace after its second Front Matter marker.
+Therefore the marker does not qualify, and the document does not have Front Matter.
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed3.md b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed3.md
new file mode 100644
index 0000000..9621704
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/front-marker-malformed3.md
@@ -0,0 +1,4 @@
+---
+name: "Aborted YAML"
+description: "The ending marker does not end with a newline, so it's invalid."
+---
\ No newline at end of file
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/oss-fuzz-42533775.md b/tests/auto/gui/text/qtextmarkdownimporter/data/oss-fuzz-42533775.md
new file mode 100644
index 0000000..04ff53a
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/oss-fuzz-42533775.md
@@ -0,0 +1 @@
+--- ---
\ No newline at end of file
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/yaml-crlf.md b/tests/auto/gui/text/qtextmarkdownimporter/data/yaml-crlf.md
new file mode 100644
index 0000000..c3e5243
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/yaml-crlf.md
@@ -0,0 +1,10 @@
+---
+name: "Venus"
+discoverer: "Galileo Galilei"
+title: "A description of the planet Venus"
+keywords:
+ - planets
+ - solar system
+ - astronomy
+---
+*Venus* is the second planet from the Sun, orbiting it every 224.7 Earth days.
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
index d9fe000..1a71b48 100644
--- a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
+++ b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
@@ -548,6 +548,7 @@
QTest::addColumn<QString>("warning");
QTest::newRow("fuzz20450") << "attempted to insert into a list that no longer exists";
QTest::newRow("fuzz20580") << "";
+ QTest::newRow("oss-fuzz-42533775") << ""; // caused a heap-buffer-overflow
}
void tst_QTextMarkdownImporter::pathological() // avoid crashing on crazy input
@@ -644,15 +645,21 @@
void tst_QTextMarkdownImporter::frontMatter_data()
{
QTest::addColumn<QString>("inputFile");
+ QTest::addColumn<int>("expectedFrontMatterSize");
QTest::addColumn<int>("expectedBlockCount");
- QTest::newRow("yaml + markdown") << QFINDTESTDATA("data/yaml.md") << 1;
- QTest::newRow("yaml only") << QFINDTESTDATA("data/yaml-only.md") << 0;
+ QTest::newRow("yaml + markdown") << QFINDTESTDATA("data/yaml.md") << 140 << 1;
+ QTest::newRow("yaml + markdown with CRLFs") << QFINDTESTDATA("data/yaml-crlf.md") << 140 << 1;
+ QTest::newRow("yaml only") << QFINDTESTDATA("data/yaml-only.md") << 59 << 0;
+ QTest::newRow("malformed 1") << QFINDTESTDATA("data/front-marker-malformed1.md") << 0 << 1;
+ QTest::newRow("malformed 2") << QFINDTESTDATA("data/front-marker-malformed2.md") << 0 << 2;
+ QTest::newRow("malformed 3") << QFINDTESTDATA("data/front-marker-malformed3.md") << 0 << 1;
}
void tst_QTextMarkdownImporter::frontMatter()
{
QFETCH(QString, inputFile);
+ QFETCH(int, expectedFrontMatterSize);
QFETCH(int, expectedBlockCount);
QFile f(inputFile);
@@ -672,7 +679,9 @@
++blockCount;
}
QCOMPARE(blockCount, expectedBlockCount); // yaml is not part of the markdown text
- QCOMPARE(doc.metaInformation(QTextDocument::FrontMatter), yaml); // without fences
+ if (expectedFrontMatterSize)
+ QCOMPARE(doc.metaInformation(QTextDocument::FrontMatter), yaml); // without fences
+ QCOMPARE(doc.metaInformation(QTextDocument::FrontMatter).size(), expectedFrontMatterSize);
}
void tst_QTextMarkdownImporter::toRawText_data()

View file

@ -0,0 +1,60 @@
From bbeccc0c22e520f46f0b33e281fa5ac85ac9c727 Mon Sep 17 00:00:00 2001
From: Mårten Nordheim <marten.nordheim@qt.io>
Date: Mon, 17 Mar 2025 14:22:11 +0100
Subject: [PATCH] QFileSystemEngine/Win: Use GetTempPath2 when available
Because the documentation for GetTempPath nows says apps should call
GetTempPath2.[0]
Starting with Windows 11[1], and recently Windows 10[2],
GetTempPath2 was added. The difference being that elevated
processes are returned a different directory. Usually
'C:\Windows\SystemTemp'.
Currently temporary files of an elevated process may be placed in a
world write-able location. GetTempPath2, by default, but can be
overridden, places it in a directory that's only accessible by SYSTEM
and administrators.
[0] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppathw#remarks
[1] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppath2w
(Minimum supported client - Windows 11 Build 22000)
[2] https://blogs.windows.com/windows-insider/2025/03/13/releasing-windows-10-build-19045-5674-to-the-release-preview-channel/
(This update enables system processes to store temporary files ...)
[ChangeLog][QtCore][Important Behavior Changes] On
Windows, generating temporary directories for processes with elevated
privileges may now return a different path with a stricter
set of permissions. Please consult Microsoft's documentation from when
they made the same change for the .NET framework:
https://support.microsoft.com/en-us/topic/gettemppath-changes-in-windows-february-cumulative-update-preview-4cc631fb-9d97-4118-ab6d-f643cd0a7259
Pick-to: 6.5 5.15
Change-Id: I5caf11151fb2f711bbc5599231f140598b3c9d03
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
(cherry picked from commit 69633bcb58e681bac5bff3744e5a2352788dc36c)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 6a684a53b371ec483b27bf243af24819be63f85f)
---
diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp
index 8300c3b..6ea1f1b 100644
--- a/src/corelib/io/qfilesystemengine_win.cpp
+++ b/src/corelib/io/qfilesystemengine_win.cpp
@@ -1635,7 +1635,15 @@
{
QString ret;
wchar_t tempPath[MAX_PATH];
- const DWORD len = GetTempPath(MAX_PATH, tempPath);
+ using GetTempPathPrototype = DWORD (WINAPI *)(DWORD, LPWSTR);
+ // We try to resolve GetTempPath2 and use that, otherwise fall back to GetTempPath:
+ static GetTempPathPrototype getTempPathW = []() {
+ const HMODULE kernel32 = GetModuleHandleW(L"kernel32.dll");
+ if (auto *func = QFunctionPointer(GetProcAddress(kernel32, "GetTempPath2W")))
+ return GetTempPathPrototype(func);
+ return GetTempPath;
+ }();
+ const DWORD len = getTempPathW(MAX_PATH, tempPath);
if (len) { // GetTempPath() can return short names, expand.
wchar_t longTempPath[MAX_PATH];
const DWORD longLen = GetLongPathName(tempPath, longTempPath, MAX_PATH);

View file

@ -0,0 +1,21 @@
diff --git a/src/corelib/io/qdataurl.cpp b/src/corelib/io/qdataurl.cpp
index 65b934b3f67..c5ecca8fb82 100644
--- a/src/corelib/io/qdataurl.cpp
+++ b/src/corelib/io/qdataurl.cpp
@@ -47,10 +47,10 @@ Q_CORE_EXPORT bool qDecodeDataUrl(const QUrl &uri, QString &mimeType, QByteArray
QLatin1StringView textPlain;
constexpr auto charset = "charset"_L1;
if (QLatin1StringView{data}.startsWith(charset, Qt::CaseInsensitive)) {
- qsizetype i = charset.size();
- while (data.at(i) == ' ')
- ++i;
- if (data.at(i) == '=')
+ QByteArrayView copy = data.sliced(charset.size());
+ while (copy.startsWith(' '))
+ copy.slice(1);
+ if (copy.startsWith('='))
textPlain = "text/plain;"_L1;
}

View file

@ -3,7 +3,7 @@
# On update rebuild all pkg with qt6-base-private-devel
pkgname=qt6-base
version=6.8.2
revision=2
revision=3
build_style=cmake
configure_args="-DINSTALL_DATADIR=share/qt6
-DINSTALL_ARCHDATADIR=lib${XBPS_TARGET_WORDSIZE}/qt6