1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QSysInfo: sanitize /etc file parsing [1/N]: properly check endsWith('"') in unquote()

Browse Source 
When 3535f46374ccf8ad966e4b266c0dbd919646fded dropped the
Q_ASSERT(end[-1] == '"') by reviewer request from the original
767beb7fff6f08f5574a193967f8f814970ac289 change, it didn't replace it
with some other check, leaving the possibility open that we'd be
passing an invalid range (end < begin) to QString::fromUtf8(), with
unknown consequences.

While technically a security problem, this has very low impact, since
the files being parsed should contain "trusted content", being
supposed to be owned by root, and not writable by mere mortals. I
hasten to add, though, that the code doesn't check permissions of the
files it reads.

The bug existed with the original Q_ASSERT, too. Namely in release
mode. And in debug mode, it would be a DOS.

Port to QByteArrayView to get the more expressive API and as a
prerequestite for follow-up (non-security) patches.

Pick-to: 6.5
Change-Id: Ib79cdad8680861d1f11b6be9234695273d0a97c2
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit c39bd86766ae4e9faa3eda89a486bd5534816b0d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2023-10-20 11:03:43 +02:00 committed by Qt Cherry-pick Bot
parent 008072cffd
commit e08534c4b6
1 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/corelib/global/qsysinfo.cpp
View File

@ -224,7 +224,7 @@ struct QUnixOSVersion
QString prettyName; // $PRETTY_NAME $DISTRIB_DESCRIPTION
};
static QString unquote(const char *begin, const char *end)
static QString unquote(QByteArrayView str)
{
// man os-release says:
// Variable assignment values must be enclosed in double
@ -235,10 +235,11 @@ static QString unquote(const char *begin, const char *end)
// All strings should be in UTF-8 format, and non-printable
// characters should not be used. It is not supported to
// concatenate multiple individually quoted strings.
if (*begin == '"')
return QString::fromUtf8(begin + 1, end - begin - 2);
return QString::fromUtf8(begin, end - begin);
if (str.size() >= 2 && str.front() == '"' && str.back() == '"')
str = str.sliced(1).chopped(1);
return QString::fromUtf8(str);
}
static QByteArray getEtcFileContent(const char *filename)
{
// we're avoiding QFile here
@ -279,19 +280,19 @@ static bool readEtcFile(QUnixOSVersion &v, const char *filename,
if (line.startsWith(idKey)) {
ptr += idKey.size();
v.productType = unquote(ptr, eol);
v.productType = unquote({ptr, eol});
continue;
}
if (line.startsWith(prettyNameKey)) {
ptr += prettyNameKey.size();
v.prettyName = unquote(ptr, eol);
v.prettyName = unquote({ptr, eol});
continue;
}
if (line.startsWith(versionKey)) {
ptr += versionKey.size();
v.productVersion = unquote(ptr, eol);
v.productVersion = unquote({ptr, eol});
continue;
}
}