diff mbox series

package/qt5/qt5base: fix CVE-2022-25255

Message ID 20220309142913.2102898-1-foss+buildroot@0leil.net
State Superseded
Headers show
Series package/qt5/qt5base: fix CVE-2022-25255 | expand

Commit Message

Quentin Schulz March 9, 2022, 2:29 p.m. UTC
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

This fixes CVE-2022-25255[1] by using a patch provided by Qt for 5.15.

The patch fails to apply because of a conflict in copyright year so it
needed some additional handling after downloading.

The patch file was added by running the following command:
wget https://download.qt.io/archive/qt/5.15/CVE-2022-25255-qprocess5-15.diff -q -O - | sed -e 's/Copyright (C) 2021 The Qt Company Ltd./Copyright (C) 2016 The Qt Company Ltd./' > 0008-CVE-2022-25255-qprocess5-15.diff.patch

[1] https://nvd.nist.gov/vuln/detail/CVE-2022-25255

Cc: Quentin Schulz <foss+buildroot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 ...008-CVE-2022-25255-qprocess5-15.diff.patch | 56 +++++++++++++++++++
 package/qt5/qt5base/qt5base.mk                |  2 +
 2 files changed, 58 insertions(+)
 create mode 100644 package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch

Comments

Arnout Vandecappelle March 10, 2022, 8:10 p.m. UTC | #1
Hi Quentin,

On 09/03/2022 15:29, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> This fixes CVE-2022-25255[1] by using a patch provided by Qt for 5.15.
> 
> The patch fails to apply because of a conflict in copyright year so it
> needed some additional handling after downloading.
> 
> The patch file was added by running the following command:
> wget https://download.qt.io/archive/qt/5.15/CVE-2022-25255-qprocess5-15.diff -q -O - | sed -e 's/Copyright (C) 2021 The Qt Company Ltd./Copyright (C) 2016 The Qt Company Ltd./' > 0008-CVE-2022-25255-qprocess5-15.diff.patch
> 
> [1] https://nvd.nist.gov/vuln/detail/CVE-2022-25255
> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>   ...008-CVE-2022-25255-qprocess5-15.diff.patch | 56 +++++++++++++++++++
>   package/qt5/qt5base/qt5base.mk                |  2 +
>   2 files changed, 58 insertions(+)
>   create mode 100644 package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
> 
> diff --git a/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
> new file mode 100644
> index 0000000000..dfab92a9ef
> --- /dev/null
> +++ b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch

  Please make the patch something that approaches git-formatted. At the very 
least, the patch itself must have a description of what it does (i.e. a commit 
message) and a Signed-off-by from you. Ideally, though, you look for the 
appropriate commit in the upstream repository [1] and use that instead of a 
random diff. In that case, add something like this at the end of the description 
part:

[Quentin: backport from upstream commit ab6915f0efb12cfe48d1f126f4a828212f853ce5 
in 6.2 branch]
Signed-off-by: ...


  Regards,
  Arnout

[1] 
https://invent.kde.org/qt/qt/qtbase/-/commit/ab6915f0efb12cfe48d1f126f4a828212f853ce5.patch


> @@ -0,0 +1,56 @@
> +--- a/src/corelib/io/qprocess_unix.cpp
> ++++ b/src/corelib/io/qprocess_unix.cpp
> +@@ -1,7 +1,7 @@
> + /****************************************************************************
> + **
> + ** Copyright (C) 2016 The Qt Company Ltd.
> +-** Copyright (C) 2016 Intel Corporation.
> ++** Copyright (C) 2022 Intel Corporation.
> + ** Contact: https://www.qt.io/licensing/
> + **
> + ** This file is part of the QtCore module of the Qt Toolkit.
> +@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
> +     // Add the program name to the argument list.
> +     argv[0] = nullptr;
> +     if (!program.contains(QLatin1Char('/'))) {
> ++        // findExecutable() returns its argument if it's an absolute path,
> ++        // otherwise it searches $PATH; returns empty if not found (we handle
> ++        // that case much later)
> +         const QString &exeFilePath = QStandardPaths::findExecutable(program);
> +-        if (!exeFilePath.isEmpty()) {
> +-            const QByteArray &tmp = QFile::encodeName(exeFilePath);
> +-            argv[0] = ::strdup(tmp.constData());
> +-        }
> +-    }
> +-    if (!argv[0])
> ++        const QByteArray &tmp = QFile::encodeName(exeFilePath);
> ++        argv[0] = ::strdup(tmp.constData());
> ++    } else {
> +         argv[0] = ::strdup(encodedProgramName.constData());
> ++    }
> +
> +     // Add every argument to the list
> +     for (int i = 0; i < arguments.count(); ++i)
> +@@ -983,15 +984,16 @@ bool QProcessPrivate::startDetached(qint64 *pid)
> +                 envp = _q_dupEnvironment(environment.d.constData()->vars, &envc);
> +             }
> +
> +-            QByteArray tmp;
> +             if (!program.contains(QLatin1Char('/'))) {
> ++                // findExecutable() returns its argument if it's an absolute path,
> ++                // otherwise it searches $PATH; returns empty if not found (we handle
> ++                // that case much later)
> +                 const QString &exeFilePath = QStandardPaths::findExecutable(program);
> +-                if (!exeFilePath.isEmpty())
> +-                    tmp = QFile::encodeName(exeFilePath);
> ++                const QByteArray &tmp = QFile::encodeName(exeFilePath);
> ++                argv[0] = ::strdup(tmp.constData());
> ++            } else {
> ++                argv[0] = ::strdup(QFile::encodeName(program));
> +             }
> +-            if (tmp.isEmpty())
> +-                tmp = QFile::encodeName(program);
> +-            argv[0] = tmp.data();
> +
> +             if (envp)
> +                 qt_safe_execve(argv[0], argv, envp);
> diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
> index ef38d03253..ac0781c430 100644
> --- a/package/qt5/qt5base/qt5base.mk
> +++ b/package/qt5/qt5base/qt5base.mk
> @@ -15,6 +15,8 @@ QT5BASE_SYNC_QT_HEADERS = YES
>   # 0010-Avoid-processing-intensive-painting-of-high-number-o.patch
>   # 0011-Improve-fix-for-avoiding-huge-number-of-tiny-dashes.patch
>   QT5BASE_IGNORE_CVES += CVE-2021-38593
> +# 0008-CVE-2022-25255-qprocess5-15.diff.patch
> +QT5BASE_IGNORE_CVES += CVE-2022-25255
>   
>   # A few comments:
>   #  * -no-pch to workaround the issue described at
Peter Seiderer March 10, 2022, 8:59 p.m. UTC | #2
Hello *,

On Thu, 10 Mar 2022 21:10:36 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:

>   Hi Quentin,
>
> On 09/03/2022 15:29, Quentin Schulz wrote:
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > This fixes CVE-2022-25255[1] by using a patch provided by Qt for 5.15.
> >
> > The patch fails to apply because of a conflict in copyright year so it
> > needed some additional handling after downloading.
> >
> > The patch file was added by running the following command:
> > wget https://download.qt.io/archive/qt/5.15/CVE-2022-25255-qprocess5-15.diff -q -O - | sed -e 's/Copyright (C) 2021 The Qt Company Ltd./Copyright (C) 2016 The Qt Company Ltd./' > 0008-CVE-2022-25255-qprocess5-15.diff.patch
> >
> > [1] https://nvd.nist.gov/vuln/detail/CVE-2022-25255
> >
> > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> >   ...008-CVE-2022-25255-qprocess5-15.diff.patch | 56 +++++++++++++++++++
> >   package/qt5/qt5base/qt5base.mk                |  2 +
> >   2 files changed, 58 insertions(+)
> >   create mode 100644 package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
> >
> > diff --git a/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
> > new file mode 100644
> > index 0000000000..dfab92a9ef
> > --- /dev/null
> > +++ b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
>
>   Please make the patch something that approaches git-formatted. At the very
> least, the patch itself must have a description of what it does (i.e. a commit
> message) and a Signed-off-by from you. Ideally, though, you look for the
> appropriate commit in the upstream repository [1] and use that instead of a
> random diff. In that case, add something like this at the end of the description
> part:
>
> [Quentin: backport from upstream commit ab6915f0efb12cfe48d1f126f4a828212f853ce5
> in 6.2 branch]
> Signed-off-by: ...
>

..or wait until [2] is merged and bump the qt5base version/kde-git-revision
(but then only applicable for 2022.02.x/master and not for the older buildroot
branches)...

Regards
Peter

[2] https://invent.kde.org/qt/qt/qtbase/-/merge_requests/115

>
>   Regards,
>   Arnout
>
> [1]
> https://invent.kde.org/qt/qt/qtbase/-/commit/ab6915f0efb12cfe48d1f126f4a828212f853ce5.patch
>
>
> > @@ -0,0 +1,56 @@
> > +--- a/src/corelib/io/qprocess_unix.cpp
> > ++++ b/src/corelib/io/qprocess_unix.cpp
> > +@@ -1,7 +1,7 @@
> > + /****************************************************************************
> > + **
> > + ** Copyright (C) 2016 The Qt Company Ltd.
> > +-** Copyright (C) 2016 Intel Corporation.
> > ++** Copyright (C) 2022 Intel Corporation.
> > + ** Contact: https://www.qt.io/licensing/
> > + **
> > + ** This file is part of the QtCore module of the Qt Toolkit.
> > +@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
> > +     // Add the program name to the argument list.
> > +     argv[0] = nullptr;
> > +     if (!program.contains(QLatin1Char('/'))) {
> > ++        // findExecutable() returns its argument if it's an absolute path,
> > ++        // otherwise it searches $PATH; returns empty if not found (we handle
> > ++        // that case much later)
> > +         const QString &exeFilePath = QStandardPaths::findExecutable(program);
> > +-        if (!exeFilePath.isEmpty()) {
> > +-            const QByteArray &tmp = QFile::encodeName(exeFilePath);
> > +-            argv[0] = ::strdup(tmp.constData());
> > +-        }
> > +-    }
> > +-    if (!argv[0])
> > ++        const QByteArray &tmp = QFile::encodeName(exeFilePath);
> > ++        argv[0] = ::strdup(tmp.constData());
> > ++    } else {
> > +         argv[0] = ::strdup(encodedProgramName.constData());
> > ++    }
> > +
> > +     // Add every argument to the list
> > +     for (int i = 0; i < arguments.count(); ++i)
> > +@@ -983,15 +984,16 @@ bool QProcessPrivate::startDetached(qint64 *pid)
> > +                 envp = _q_dupEnvironment(environment.d.constData()->vars, &envc);
> > +             }
> > +
> > +-            QByteArray tmp;
> > +             if (!program.contains(QLatin1Char('/'))) {
> > ++                // findExecutable() returns its argument if it's an absolute path,
> > ++                // otherwise it searches $PATH; returns empty if not found (we handle
> > ++                // that case much later)
> > +                 const QString &exeFilePath = QStandardPaths::findExecutable(program);
> > +-                if (!exeFilePath.isEmpty())
> > +-                    tmp = QFile::encodeName(exeFilePath);
> > ++                const QByteArray &tmp = QFile::encodeName(exeFilePath);
> > ++                argv[0] = ::strdup(tmp.constData());
> > ++            } else {
> > ++                argv[0] = ::strdup(QFile::encodeName(program));
> > +             }
> > +-            if (tmp.isEmpty())
> > +-                tmp = QFile::encodeName(program);
> > +-            argv[0] = tmp.data();
> > +
> > +             if (envp)
> > +                 qt_safe_execve(argv[0], argv, envp);
> > diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
> > index ef38d03253..ac0781c430 100644
> > --- a/package/qt5/qt5base/qt5base.mk
> > +++ b/package/qt5/qt5base/qt5base.mk
> > @@ -15,6 +15,8 @@ QT5BASE_SYNC_QT_HEADERS = YES
> >   # 0010-Avoid-processing-intensive-painting-of-high-number-o.patch
> >   # 0011-Improve-fix-for-avoiding-huge-number-of-tiny-dashes.patch
> >   QT5BASE_IGNORE_CVES += CVE-2021-38593
> > +# 0008-CVE-2022-25255-qprocess5-15.diff.patch
> > +QT5BASE_IGNORE_CVES += CVE-2022-25255
> >
> >   # A few comments:
> >   #  * -no-pch to workaround the issue described at
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Quentin Schulz March 11, 2022, 10:09 a.m. UTC | #3
Hi all,

On 3/10/22 21:59, Peter Seiderer wrote:
> Hello *,
> 
> On Thu, 10 Mar 2022 21:10:36 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>    Hi Quentin,
>>
>> On 09/03/2022 15:29, Quentin Schulz wrote:
>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>
>>> This fixes CVE-2022-25255[1] by using a patch provided by Qt for 5.15.
>>>
>>> The patch fails to apply because of a conflict in copyright year so it
>>> needed some additional handling after downloading.
>>>
>>> The patch file was added by running the following command:
>>> wget https://urldefense.proofpoint.com/v2/url?u=https-3A__download.qt.io_archive_qt_5.15_CVE-2D2022-2D25255-2Dqprocess5-2D15.diff&d=DwIFAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=BsFMymMtfNBibxvKyvdJPDHFdo422rhoYyW1yKTW6AhrH0dKs7xDb-efWy6vBNAT&s=-6cfccEeLqsLUibYZcNYVLGYEN_CxlShGyX6MzeFYX8&e=  -q -O - | sed -e 's/Copyright (C) 2021 The Qt Company Ltd./Copyright (C) 2016 The Qt Company Ltd./' > 0008-CVE-2022-25255-qprocess5-15.diff.patch
>>>
>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__nvd.nist.gov_vuln_detail_CVE-2D2022-2D25255&d=DwIFAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=BsFMymMtfNBibxvKyvdJPDHFdo422rhoYyW1yKTW6AhrH0dKs7xDb-efWy6vBNAT&s=qywJU5cDeT4YRPIoWShq1zOH4t4wlihP8FgpBlCIvoE&e=
>>>
>>> Cc: Quentin Schulz <foss+buildroot@0leil.net>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>> ---
>>>    ...008-CVE-2022-25255-qprocess5-15.diff.patch | 56 +++++++++++++++++++
>>>    package/qt5/qt5base/qt5base.mk                |  2 +
>>>    2 files changed, 58 insertions(+)
>>>    create mode 100644 package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
>>>
>>> diff --git a/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
>>> new file mode 100644
>>> index 0000000000..dfab92a9ef
>>> --- /dev/null
>>> +++ b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
>>
>>    Please make the patch something that approaches git-formatted. At the very
>> least, the patch itself must have a description of what it does (i.e. a commit
>> message) and a Signed-off-by from you. Ideally, though, you look for the
>> appropriate commit in the upstream repository [1] and use that instead of a
>> random diff. In that case, add something like this at the end of the description

Not technically a random diff. It's hosted on an official Qt website 
where the tarballs used to be fetched from.

There's no commit log because well, there is no commit log. I cannot 
invent it :) The commit you point to is completely different from the 
diff in this patch, which makes me uncomfortable with stealing the 
commit log and apply it to this diff.

Also, there is no commit available for 5.15 branch (at least publicly ?) 
yet.

See the discussion for 5.12 LTS branch:
https://codereview.qt-project.org/c/qt/qtbase/+/396020

Specifically:

"5.15 has ended up needing even more path corrections for helpers. Will 
likely be needed here as well"

"Yeah, I have, so far, just manually changed tests for 5.15. But there's 
quite a few and took a long time to get them all, and it will have to be 
repeated for other modules.
I was thinking about this too, but then, changing the test-runner may 
break other things as well. But it's worth looking into"

and finally, this patchset was abandoned. I quickly checked this 
person's other commits on Qt Gerrit but found nothing interesting.

I could have done a better job at writing my commit log and give all 
this info in it to explain why such an unusual patch should/could be 
included.

>> part:
>>
>> [Quentin: backport from upstream commit ab6915f0efb12cfe48d1f126f4a828212f853ce5
>> in 6.2 branch]
>> Signed-off-by: ...
>>
> 
> ..or wait until [2] is merged and bump the qt5base version/kde-git-revision
> (but then only applicable for 2022.02.x/master and not for the older buildroot
> branches)...
> 

I needed this fixed now and that is the only reasonable way to fix the 
CVE at the moment. If the Buildroot project prefers to wait for the KDE 
PR to be merged, that's another story and I completely understand :)

No hard feelings if this patch isn't merged, I understand it's not 
really on-par with Buildroot policies/best practices as highlighted by 
Arnout.

Regards,
Quentin
Quentin Schulz March 17, 2022, 4:40 p.m. UTC | #4
Hi all,

Please ignore this patch because it is superseded by:
http://lists.busybox.net/pipermail/buildroot/2022-March/638898.html

Thanks,
Quentin
Arnout Vandecappelle March 17, 2022, 8:39 p.m. UTC | #5
On 17/03/2022 17:40, Quentin Schulz wrote:
> Hi all,
> 
> Please ignore this patch because it is superseded by:
> http://lists.busybox.net/pipermail/buildroot/2022-March/638898.html

  Instead of replying to the list, it's nicer if you create an account on 
patchwork [1] and update the patch state to Superseded yourself.

  Also, in the future please add the version in the subject of the new patch 
(series) using the -v2 option to git format-patch, and add a patch changelog 
below the --- line. See [2].


  Regards,
  Arnout

[1] https://patchwork.ozlabs.org/project/buildroot/list/
[2] https://buildroot.org/downloads/manual/manual.html#_patch_revision_changelog


> 
> Thanks,
> Quentin
diff mbox series

Patch

diff --git a/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
new file mode 100644
index 0000000000..dfab92a9ef
--- /dev/null
+++ b/package/qt5/qt5base/0008-CVE-2022-25255-qprocess5-15.diff.patch
@@ -0,0 +1,56 @@ 
+--- a/src/corelib/io/qprocess_unix.cpp
++++ b/src/corelib/io/qprocess_unix.cpp
+@@ -1,7 +1,7 @@
+ /****************************************************************************
+ **
+ ** Copyright (C) 2016 The Qt Company Ltd.
+-** Copyright (C) 2016 Intel Corporation.
++** Copyright (C) 2022 Intel Corporation.
+ ** Contact: https://www.qt.io/licensing/
+ **
+ ** This file is part of the QtCore module of the Qt Toolkit.
+@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
+     // Add the program name to the argument list.
+     argv[0] = nullptr;
+     if (!program.contains(QLatin1Char('/'))) {
++        // findExecutable() returns its argument if it's an absolute path,
++        // otherwise it searches $PATH; returns empty if not found (we handle
++        // that case much later)
+         const QString &exeFilePath = QStandardPaths::findExecutable(program);
+-        if (!exeFilePath.isEmpty()) {
+-            const QByteArray &tmp = QFile::encodeName(exeFilePath);
+-            argv[0] = ::strdup(tmp.constData());
+-        }
+-    }
+-    if (!argv[0])
++        const QByteArray &tmp = QFile::encodeName(exeFilePath);
++        argv[0] = ::strdup(tmp.constData());
++    } else {
+         argv[0] = ::strdup(encodedProgramName.constData());
++    }
+
+     // Add every argument to the list
+     for (int i = 0; i < arguments.count(); ++i)
+@@ -983,15 +984,16 @@ bool QProcessPrivate::startDetached(qint64 *pid)
+                 envp = _q_dupEnvironment(environment.d.constData()->vars, &envc);
+             }
+
+-            QByteArray tmp;
+             if (!program.contains(QLatin1Char('/'))) {
++                // findExecutable() returns its argument if it's an absolute path,
++                // otherwise it searches $PATH; returns empty if not found (we handle
++                // that case much later)
+                 const QString &exeFilePath = QStandardPaths::findExecutable(program);
+-                if (!exeFilePath.isEmpty())
+-                    tmp = QFile::encodeName(exeFilePath);
++                const QByteArray &tmp = QFile::encodeName(exeFilePath);
++                argv[0] = ::strdup(tmp.constData());
++            } else {
++                argv[0] = ::strdup(QFile::encodeName(program));
+             }
+-            if (tmp.isEmpty())
+-                tmp = QFile::encodeName(program);
+-            argv[0] = tmp.data();
+
+             if (envp)
+                 qt_safe_execve(argv[0], argv, envp);
diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
index ef38d03253..ac0781c430 100644
--- a/package/qt5/qt5base/qt5base.mk
+++ b/package/qt5/qt5base/qt5base.mk
@@ -15,6 +15,8 @@  QT5BASE_SYNC_QT_HEADERS = YES
 # 0010-Avoid-processing-intensive-painting-of-high-number-o.patch
 # 0011-Improve-fix-for-avoiding-huge-number-of-tiny-dashes.patch
 QT5BASE_IGNORE_CVES += CVE-2021-38593
+# 0008-CVE-2022-25255-qprocess5-15.diff.patch
+QT5BASE_IGNORE_CVES += CVE-2022-25255
 
 # A few comments:
 #  * -no-pch to workaround the issue described at