Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Salvatore Bonaccorso
Hi Norbert,

On Thu, Nov 05, 2020 at 09:15:15PM +0900, Norbert Preining wrote:
> Hi Salvatore,
> 
> On Thu, 05 Nov 2020, Salvatore Bonaccorso wrote:
> > 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.
> 
> Ah, ok, didn't know that. Fine with me!
> 
> > 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.
> 
> Thanks, that is great. I will trash my branch and make a new one based
> on your commit. I wait with tagging until I see the package appear.

DSA 4783-1 released.

Regards,
Salvatore



Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Norbert Preining
Hi Salvatore,

On Thu, 05 Nov 2020, Salvatore Bonaccorso wrote:
> 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.

Ah, ok, didn't know that. Fine with me!

> 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.

Thanks, that is great. I will trash my branch and make a new one based
on your commit. I wait with tagging until I see the package appear.

Best regards

Norbert

--
PREINING Norbert  https://www.preining.info
Accelia Inc. + IFMGA ProGuide + TU Wien + JAIST + TeX Live + Debian Dev
GPG: 0x860CDC13   fp: F7D8 A928 26E3 16A1 9FA0 ACF0 6CAC A448 860C DC13



Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Salvatore Bonaccorso
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.
> 
> A  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 
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 ..c6a8808770e7
--- /dev/null
+++ b/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff
@@ -0,0 +1,93 @@
+From: Fabian Vogt 
+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 ) {
++bool XorgDisplayServer::addCookie(const QString ) {
+ // 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")) {
++

Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Norbert Preining
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.

A  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 ...

Is there anything regarding buster that is still necessary?

> Fixing this via unstable via directly 0.19 sounds great, thank you.

That is coming in in short time.

Thanks

Norbert

--
PREINING Norbert  https://www.preining.info
Accelia Inc. + IFMGA ProGuide + TU Wien + JAIST + TeX Live + Debian Dev
GPG: 0x860CDC13   fp: F7D8 A928 26E3 16A1 9FA0 ACF0 6CAC A448 860C DC13



Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Salvatore Bonaccorso
Hi Norbert,

On Thu, Nov 05, 2020 at 08:26:07PM +0900, Norbert Preining wrote:
> Hi Salvatore, hi FTP Master,
> 
> @Salvatore: thanks for the NMU preparation. We are now preparing a fix
> for unstable via version 0.19, and at the same time I thought I upload
> to buster-security, based on your patch,
> 
> But, uploading to security-master with dput I got the following answer:
> 
> On Thu, 05 Nov 2020, Debian FTP Masters wrote:
> > sddm_0.18.0-1+deb10u1.dsc: Does not match file already existing in the pool.
> 
> Do you or ftpmaster could explain me what I did wrong?
> 
> The included files are
> Checksums-Sha1:
>  f8d882dbf4cf377fa0c7a4277a56b7f7c25e2a64 2334 sddm_0.18.0-1+deb10u1.dsc
>  a33d316b613a52b2af435c3516ed9abac7ea34d5 52864 
> sddm_0.18.0-1+deb10u1.debian.tar.xz
>  5ed47e94dc64af78fe960358b8b009afb840e16a 13613 
> sddm_0.18.0-1+deb10u1_source.buildinfo
> and the upload was a source-only to Distribution: buster-security.

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.

Fixing this via unstable via directly 0.19 sounds great, thank you.

Regards,
Salvatore



Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-05 Thread Norbert Preining
Hi Salvatore, hi FTP Master,

@Salvatore: thanks for the NMU preparation. We are now preparing a fix
for unstable via version 0.19, and at the same time I thought I upload
to buster-security, based on your patch,

But, uploading to security-master with dput I got the following answer:

On Thu, 05 Nov 2020, Debian FTP Masters wrote:
> sddm_0.18.0-1+deb10u1.dsc: Does not match file already existing in the pool.

Do you or ftpmaster could explain me what I did wrong?

The included files are
Checksums-Sha1:
 f8d882dbf4cf377fa0c7a4277a56b7f7c25e2a64 2334 sddm_0.18.0-1+deb10u1.dsc
 a33d316b613a52b2af435c3516ed9abac7ea34d5 52864 
sddm_0.18.0-1+deb10u1.debian.tar.xz
 5ed47e94dc64af78fe960358b8b009afb840e16a 13613 
sddm_0.18.0-1+deb10u1_source.buildinfo
and the upload was a source-only to Distribution: buster-security.

Thanks

Norbert

--
PREINING Norbert  https://www.preining.info
Accelia Inc. + IFMGA ProGuide + TU Wien + JAIST + TeX Live + Debian Dev
GPG: 0x860CDC13   fp: F7D8 A928 26E3 16A1 9FA0 ACF0 6CAC A448 860C DC13



Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-04 Thread Salvatore Bonaccorso
Hi,

On Wed, Nov 04, 2020 at 01:52:12PM +0100, Salvatore Bonaccorso wrote:
> Source: sddm
> Version: 0.18.1-1
> Severity: grave
> Tags: security upstream
> Justification: user security hole
> X-Debbugs-Cc: car...@debian.org, Debian Security Team 
> 
> 
> Hi,
> 
> The following vulnerability was published for sddm.
> 
> CVE-2020-28049[0]:
> | local privilege escalation due to race condition in creation of the
> | Xauthority file
> 
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
> 
> For further information see:
> 
> [0] https://security-tracker.debian.org/tracker/CVE-2020-28049
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28049
> [1] 
> https://github.com/sddm/sddm/commit/be202f533ab98a684c6a007e8d5b4357846bc222
> [2] https://bugzilla.suse.com/show_bug.cgi?id=1177201
> [3] https://www.openwall.com/lists/oss-security/2020/11/04/2

Attached the debdiff as to be used for the buster-security update.

Regards,
Salvatore
diff -Nru sddm-0.18.0/debian/changelog sddm-0.18.0/debian/changelog
--- sddm-0.18.0/debian/changelog2018-07-22 13:26:44.0 +0200
+++ sddm-0.18.0/debian/changelog2020-11-04 15:29:27.0 +0100
@@ -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   Wed, 04 Nov 2020 15:29:27 +0100
+
 sddm (0.18.0-1) unstable; urgency=medium
 
   [ Simon Quigley ]
diff -Nru 
sddm-0.18.0/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff 
sddm-0.18.0/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff
--- 
sddm-0.18.0/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff   
1970-01-01 01:00:00.0 +0100
+++ 
sddm-0.18.0/debian/patches/06_Fix-X-not-having-access-control-on-startup.diff   
2020-11-04 15:29:27.0 +0100
@@ -0,0 +1,93 @@
+From: Fabian Vogt 
+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 ) {
++bool XorgDisplayServer::addCookie(const QString ) {
+ // 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() 

Bug#973748: sddm: CVE-2020-28049: local privilege escalation due to race condition in creation of the Xauthority file

2020-11-04 Thread Salvatore Bonaccorso
Source: sddm
Version: 0.18.1-1
Severity: grave
Tags: security upstream
Justification: user security hole
X-Debbugs-Cc: car...@debian.org, Debian Security Team 

Hi,

The following vulnerability was published for sddm.

CVE-2020-28049[0]:
| local privilege escalation due to race condition in creation of the
| Xauthority file

If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2020-28049
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28049
[1] https://github.com/sddm/sddm/commit/be202f533ab98a684c6a007e8d5b4357846bc222
[2] https://bugzilla.suse.com/show_bug.cgi?id=1177201
[3] https://www.openwall.com/lists/oss-security/2020/11/04/2

Please adjust the affected versions in the BTS as needed.

Regards,
Salvatore