Add a bunch of patches to fix CVE-2009-1756, also reported in debian bz

#529306 & FreeBSD PR134801 :
The security issue is caused by slim generating the X authority file
by passing the X authority cookie via the command line to "xauth".
This can be exploited to disclose the X authority cookie by consulting
the process list and e.g. gain access the user's display.
While here, use slightly better random seeding for cookie generation.

Patches adapted from the ones provided to debian/FreeBSD by Eygene Ryabinkin <rea@codelabs.ru>
This commit is contained in:
landry 2009-09-04 20:24:25 +00:00
parent b19a3cca3a
commit b6a02eab6a
4 changed files with 210 additions and 1 deletions

View File

@ -1,8 +1,9 @@
# $OpenBSD: Makefile,v 1.5 2008/11/05 13:36:41 pea Exp $
# $OpenBSD: Makefile,v 1.6 2009/09/04 20:24:25 landry Exp $
COMMENT= simple login manager
DISTNAME= slim-1.3.1
PKGNAME= ${DISTNAME}p0
CATEGORIES= x11
HOMEPAGE= http://slim.berlios.de/

View File

@ -0,0 +1,154 @@
$OpenBSD: patch-app_cpp,v 1.1 2009/09/04 20:24:25 landry Exp $
Slim used to spawn 'xauth add . <COOKIE>' via the system() call, so the
cookie itself was visible. On multi-user system one can poll for the
xauth processes via ps and gather cookies for X sessions.
fixes CVE-2009-1756
--- app.cpp.orig Fri Sep 4 21:58:08 2009
+++ app.cpp Fri Sep 4 22:07:29 2009
@@ -32,6 +32,62 @@
using namespace std;
+/* Code taken from http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306 */
+/* From: Eygene Ryabinkin <rea@shadow.codelabs.ru> */
+#include <time.h>
+#include <stdlib.h>
+
+/*
+ * Adds the given cookie to the specified Xauthority file.
+ * Returns true on success, false on fault.
+ */
+bool Util::add_mcookie(const std::string &mcookie, const char *display,
+ const std::string &xauth_cmd, const std::string &authfile)
+{
+ FILE *fp;
+ std::string cmd = xauth_cmd + " -f " + authfile + " -q";
+
+ fp = popen(cmd.c_str(), "w");
+ if (!fp)
+ return false;
+ fprintf(fp, "remove %s\n", display);
+ fprintf(fp, "add %s %s %s\n", display, ".", mcookie.c_str());
+ fprintf(fp, "exit\n");
+
+ pclose(fp);
+ return true;
+}
+/*
+ * Interface for random number generator. Just now it uses ordinary
+ * random/srandom routines and serves as a wrapper for them.
+ */
+void Util::srandom(unsigned long seed)
+{
+ ::srandom(seed);
+}
+
+long Util::random(void)
+{
+ return ::random();
+}
+
+/*
+ * Makes seed for the srandom() using "random" values obtained from
+ * getpid(), time(NULL) and others.
+ */
+long Util::makeseed(void)
+{
+ struct timespec ts;
+ long pid = getpid();
+ long tm = time(NULL);
+
+ if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
+ ts.tv_sec = ts.tv_nsec = 0;
+ }
+
+ return pid + tm + (ts.tv_sec ^ ts.tv_nsec);
+}
+
#ifdef USE_PAM
#include <string>
@@ -131,12 +187,12 @@ void User1Signal(int sig) {
App::App(int argc, char** argv):
pam(conv, static_cast<void*>(&LoginPanel)){
#else
-App::App(int argc, char** argv){
+App::App(int argc, char** argv) : mcookiesize(32) {
#endif
int tmp;
ServerPID = -1;
testing = false;
- mcookie = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+ mcookie = string(App::mcookiesize, 'a');
daemonmode = false;
force_nodaemon = false;
firstlogin = true;
@@ -1127,13 +1183,13 @@ string App::findValidRandomTheme(const string& set)
name = name.substr(0, name.length() - 1);
}
- srandom(getpid()+time(NULL));
+ Util::srandom(Util::makeseed());
vector<string> themes;
string themefile;
Cfg::split(themes, name, ',');
do {
- int sel = random() % themes.size();
+ int sel = Util::random() % themes.size();
name = Cfg::Trim(themes[sel]);
themefile = string(THEMESDIR) +"/" + name + THEMESFILE;
@@ -1159,34 +1215,32 @@ void App::replaceVariables(string& input,
}
}
-
+/*
+ * We rely on the fact that all bits generated by Util::random()
+ * are usable, so we are taking full words from its output.
+ */
void App::CreateServerAuth() {
/* create mit cookie */
- int i, r;
- int hexcount = 0;
- string authfile;
- string cmd;
+ uint16_t word;
+ uint8_t hi, lo;
+ int i;
+ string authfile;
const char *digits = "0123456789abcdef";
- srand( time(NULL) );
- for ( i = 0; i < 31; i++ ) {
- r = rand()%16;
- mcookie[i] = digits[r];
- if (r>9)
- hexcount++;
+ Util::srandom(Util::makeseed());
+ for (i = 0; i < App::mcookiesize; i+=4) {
+ word = Util::random() & 0xffff;
+ lo = word & 0xff;
+ hi = word >> 8;
+ mcookie[i] = digits[lo & 0x0f];
+ mcookie[i+1] = digits[lo >> 4];
+ mcookie[i+2] = digits[hi & 0x0f];
+ mcookie[i+3] = digits[hi >> 4];
}
- /* MIT-COOKIE: even occurrences of digits and hex digits */
- if ((hexcount%2) == 0) {
- r = rand()%10;
- } else {
- r = rand()%5+10;
- }
- mcookie[31] = digits[r];
/* reinitialize auth file */
authfile = cfg->getOption("authfile");
remove(authfile.c_str());
putenv(StrConcat("XAUTHORITY=", authfile.c_str()));
- cmd = cfg->getOption("xauth_path") + " -q -f " + authfile + " add :0 . " + mcookie;
- system(cmd.c_str());
+ Util::add_mcookie(mcookie, ":0", cfg->getOption("xauth_path"), authfile);
}
char* App::StrConcat(const char* str1, const char* str2) {

View File

@ -0,0 +1,29 @@
$OpenBSD: patch-app_h,v 1.1 2009/09/04 20:24:25 landry Exp $
--- app.h.orig Fri Sep 4 21:55:48 2009
+++ app.h Fri Sep 4 22:04:27 2009
@@ -28,6 +28,16 @@
#include "PAM.h"
#endif
+#include <string>
+
+namespace Util {
+ bool add_mcookie(const std::string &mcookie, const char *display,
+ const std::string &xauth_cmd, const std::string &authfile);
+ void srandom(unsigned long seed);
+ long random(void);
+ long makeseed(void);
+};
+
class App {
public:
App(int argc, char** argv);
@@ -101,6 +111,8 @@ class App { (private)
std::string themeName;
std::string mcookie;
+
+ const int mcookiesize;
};

View File

@ -0,0 +1,25 @@
$OpenBSD: patch-switchuser_cpp,v 1.1 2009/09/04 20:24:25 landry Exp $
--- switchuser.cpp.orig Fri Sep 4 22:01:46 2009
+++ switchuser.cpp Fri Sep 4 22:02:32 2009
@@ -10,7 +10,7 @@
*/
#include "switchuser.h"
-
+#include "app.h"
using namespace std;
SwitchUser::SwitchUser(struct passwd *pw, Cfg *c, const string& display,
@@ -53,10 +53,9 @@ void SwitchUser::Execute(const char* cmd) {
}
void SwitchUser::SetClientAuth(const char* mcookie) {
- int r;
+ bool r;
string home = string(Pw->pw_dir);
string authfile = home + "/.Xauthority";
remove(authfile.c_str());
- string cmd = cfg->getOption("xauth_path") + " -q -f " + authfile + " add :0 . " + mcookie;
- r = system(cmd.c_str());
+ r = Util::add_mcookie(mcookie, ":0", cfg->getOption("xauth_path"), authfile);
}