Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file
Hi Norbert,
On Thu, Nov 05, 2020 at 08:55:40PM +0900, Norbert Preining wrote:
> Hi Salvatore,
>
> > That is because I did already upload the upload yesterday as with the
> > debdiff attached to the bugreport. But we (Moritz was testing as well)
> > wanted to further test the upload first before releasing the DSA.
>
> Ahhhh .... ok, that explains it. Didn't see any message about it, so I
> guessed you were waiting for us. I also didn't see anything on
> tracker.debian.org, so I thought I will do it ...
Sorry this was badly formulated on my end then :(. The intention was
to day, this is the debdiff I just used for the upload. tracker.d.o
does not show it yet because the packages are sitting in the embargoed
policy queue on security-master so not yet pushed out to the archive.
I will do it this afternoon or tonight the latest once I got test
feedback from Moritz as well.
> Is there anything regarding buster that is still necessary?
No not for buster-security. But I can provide you the two commits for
the packaging repo if you prefer that instead of importing the dsc.
They are attached.
> > Fixing this via unstable via directly 0.19 sounds great, thank you.
>
> That is coming in in short time.
Thank you for your work on this update (and in general for the
package).
Regards,
Salvatore
>From e2fceb114a975775fd64dd064e4b7be3dee5cd1f Mon Sep 17 00:00:00 2001
From: Salvatore Bonaccorso <carnil@debian.org>
Date: Wed, 4 Nov 2020 15:28:22 +0100
Subject: [PATCH 1/2] Fix X not having access control on startup
(CVE-2020-28049)
Closes: #973748
---
...-not-having-access-control-on-startup.diff | 93 +++++++++++++++++++
debian/patches/series | 1 +
2 files changed, 94 insertions(+)
create mode 100644 debian/patches/06_Fix-X-not-having-access-control-on-startup.diff
diff --git a/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff b/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff
new file mode 100644
index 000000000000..c6a8808770e7
--- /dev/null
+++ b/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff
@@ -0,0 +1,93 @@
+From: Fabian Vogt <fabian@ritter-vogt.de>
+Date: Tue, 6 Oct 2020 21:21:38 +0200
+Subject: Fix X not having access control on startup
+Origin: https://github.com/sddm/sddm/commit/be202f533ab98a684c6a007e8d5b4357846bc222
+Bug: https://bugzilla.suse.com/show_bug.cgi?id=1177201
+Bug-Debian: https://bugs.debian.org/973748
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2020-28049
+
+If the auth file is empty, X allows any local application (= any user on the
+system) to connect. This is currently the case until X wrote the display
+number to sddm and sddm used that to write the entry into the file.
+To work around this chicken-and-egg problem, make use of the fact that X
+doesn't actually look at the display number in the passed auth file and just
+use :0 unconditionally. Also make sure that writing the entry was actually
+successful.
+
+CVE-2020-28049
+---
+ src/daemon/XorgDisplayServer.cpp | 25 ++++++++++++++++++++-----
+ src/daemon/XorgDisplayServer.h | 2 +-
+ 2 files changed, 21 insertions(+), 6 deletions(-)
+
+--- a/src/daemon/XorgDisplayServer.cpp
++++ b/src/daemon/XorgDisplayServer.cpp
+@@ -87,7 +87,7 @@ namespace SDDM {
+ return m_cookie;
+ }
+
+- void XorgDisplayServer::addCookie(const QString &file) {
++ bool XorgDisplayServer::addCookie(const QString &file) {
+ // log message
+ qDebug() << "Adding cookie to" << file;
+
+@@ -103,13 +103,13 @@ namespace SDDM {
+
+ // check file
+ if (!fp)
+- return;
++ return false;
+ fprintf(fp, "remove %s\n", qPrintable(m_display));
+ fprintf(fp, "add %s . %s\n", qPrintable(m_display), qPrintable(m_cookie));
+ fprintf(fp, "exit\n");
+
+ // close pipe
+- pclose(fp);
++ return pclose(fp) == 0;
+ }
+
+ bool XorgDisplayServer::start() {
+@@ -126,6 +126,15 @@ namespace SDDM {
+ // log message
+ qDebug() << "Display server starting...";
+
++ // generate auth file.
++ // For the X server's copy, the display number doesn't matter.
++ // An empty file would result in no access control!
++ m_display = QStringLiteral(":0");
++ if(!addCookie(m_authPath)) {
++ qCritical() << "Failed to write xauth file";
++ return false;
++ }
++
+ if (daemonApp->testing()) {
+ QStringList args;
+ args << m_display << QStringLiteral("-ac") << QStringLiteral("-br") << QStringLiteral("-noreset") << QStringLiteral("-screen") << QStringLiteral("800x600");
+@@ -210,8 +219,14 @@ namespace SDDM {
+ emit started();
+ }
+
+- // generate auth file
+- addCookie(m_authPath);
++ // The file is also used by the greeter, which does care about the
++ // display number. Write the proper entry, if it's different.
++ if(m_display != QStringLiteral(":0")) {
++ if(!addCookie(m_authPath)) {
++ qCritical() << "Failed to write xauth file";
++ return false;
++ }
++ }
+ changeOwner(m_authPath);
+
+ // set flag
+--- a/src/daemon/XorgDisplayServer.h
++++ b/src/daemon/XorgDisplayServer.h
+@@ -40,7 +40,7 @@ namespace SDDM {
+
+ const QString &cookie() const;
+
+- void addCookie(const QString &file);
++ bool addCookie(const QString &file);
+
+ public slots:
+ bool start();
diff --git a/debian/patches/series b/debian/patches/series
index 356b8315ccd1..a0885abbe44c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,3 +3,4 @@
03_vt7-minimum-vt.diff
04_set_default_path.diff
05_add_debian_themes.diff
+06_Fix-X-not-having-access-control-on-startup.diff
--
2.29.1
>From 7fd2c46fd8847e42924db8937d58c1f824dd7afa Mon Sep 17 00:00:00 2001
From: Salvatore Bonaccorso <carnil@debian.org>
Date: Wed, 4 Nov 2020 15:29:49 +0100
Subject: [PATCH 2/2] Prepare changelog for release
Gbp-Dch: Ignore
---
debian/changelog | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 19cf57ea1d00..61f743d92e90 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+sddm (0.18.0-1+deb10u1) buster-security; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Fix X not having access control on startup (CVE-2020-28049)
+ (Closes: #973748)
+
+ -- Salvatore Bonaccorso <carnil@debian.org> Wed, 04 Nov 2020 15:29:27 +0100
+
sddm (0.18.0-1) unstable; urgency=medium
[ Simon Quigley ]
--
2.29.1
Reply to: