1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
|
From be202f533ab98a684c6a007e8d5b4357846bc222 Mon Sep 17 00:00:00 2001
From: Fabian Vogt <fabian@ritter-vogt.de>
Date: Tue, 6 Oct 2020 21:21:38 +0200
Subject: [PATCH] Fix X not having access control on startup
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(-)
diff --git a/src/daemon/XorgDisplayServer.cpp b/src/daemon/XorgDisplayServer.cpp
index d04f6344..df685b2d 100644
--- a/src/daemon/XorgDisplayServer.cpp
+++ b/src/daemon/XorgDisplayServer.cpp
@@ -88,7 +88,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;
@@ -104,13 +104,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() {
@@ -127,6 +127,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;
QDir x11socketDir(QStringLiteral("/tmp/.X11-unix"));
@@ -217,8 +226,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
diff --git a/src/daemon/XorgDisplayServer.h b/src/daemon/XorgDisplayServer.h
index d2bdf6d4..e97a0b53 100644
--- 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();
|
