Nico Golde discovered insecure transfer of X magic cookie to the .Xauthority file: [1] Fix: The following diff adds two patches: one that fixes original issue and another that makes SLiM to use random()/srandom() throughout the code and use slightly better seeding for cookie generation. New port works on two of my workstations for two days. The following VuXML entry should be evaluated and added: <vuln vid="b2c69e02-4689-11de-910d-001b77d09812"> <topic>Slim -- local disclosure of X authority magic cookie</topic> <affects> <package> <name>slim</name> <range><lt>1.3.1_3</lt></range> </package> </affects> <description> <body xmlns="http://www.w3.org/1999/xhtml"> <p>Secunia reports:</p> <blockquote cite="http://secunia.com/advisories/35132/"> <p>A security issue has been reported in SLiM, which can be exploited by malicious, local users to disclose sensitive information.</p> <p>The security issue is caused due to the application 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.</p> </blockquote> </body> </description> <references> <cvename>CVE-2009-1756</cvename> <bid>35015</bid> <url>http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306</url> </references> <dates> <discovery>2009-05-20</discovery> <entry>TODAY</entry> </dates> </vuln> --- vuln.xml ends here -----i2Z0HQS6NcTwZ1wmuY3KEbyqmUlcHf9pRfU6TF0Bszf4PC3p Content-Type: text/plain; name="fix-visible-mcookie.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="fix-visible-mcookie.diff" From a83e8761cdf36103748ee1df36c3e2eea8997faa Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> Date: Thu, 21 May 2009 17:30:43 +0400 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. The second change uses random()/srandom() instead of old and poor rand()/srand() for cookie generation. Initial seed is made a bit more unpredictable by using combination of process PID, current time and system uptime. Signed-off-by: Eygene Ryabinkin <rea-fbsd@codelabs.ru> --- x11/slim/Makefile | 2 +- x11/slim/files/patch-000-fix-visible-mcookie | 192 ++++++++++++++++++++++++ x11/slim/files/patch-001-random-routines.diff | 193 +++++++++++++++++++++++++ x11/slim/files/patch-Makefile.freebsd | 12 +- 4 files changed, 392 insertions(+), 7 deletions(-) create mode 100644 x11/slim/files/patch-000-fix-visible-mcookie create mode 100644 x11/slim/files/patch-001-random-routines.diff diff --git a/x11/slim/Makefile b/x11/slim/Makefile index 7e94f0f..7b0150e 100644 --- a/x11/slim/Makefile +++ b/x11/slim/Makefile @@ -7,7 +7,7 @@ PORTNAME= slim PORTVERSION= 1.3.1 -PORTREVISION= 2 +PORTREVISION= 3 CATEGORIES= x11 MASTER_SITES= ${MASTER_SITE_BERLIOS} \ http://depot.fsck.ch/mirror/distfiles/ diff --git a/x11/slim/files/patch-000-fix-visible-mcookie b/x11/slim/files/patch-000-fix-visible-mcookie new file mode 100644 index 0000000..77cd5c1 --- /dev/null +++ b/x11/slim/files/patch-000-fix-visible-mcookie @@ -0,0 +1,192 @@ +From 72625a9dacfbd448ba7a84725d66bb2bfc9801f0 Mon Sep 17 00:00:00 2001 +From: Eygene Ryabinkin <rea@codelabs.ru> +Date: Wed, 20 May 2009 18:44:57 +0400 +Subject: [PATCH] Do not specify magic cookie for xauth in the xauth command line + +Instead, open xauth as a pipe and feed commands via its stdin. + +Signed-off-by: Eygene Ryabinkin <rea@codelabs.ru> +--- + Makefile | 3 ++- + Makefile.freebsd | 3 ++- + Makefile.netbsd | 3 ++- + Makefile.openbsd | 3 ++- + app.cpp | 5 +++-- + switchuser.cpp | 7 ++++--- + util.cpp | 32 ++++++++++++++++++++++++++++++++ + util.h | 19 +++++++++++++++++++ + 8 files changed, 66 insertions(+), 9 deletions(-) + create mode 100644 util.cpp + create mode 100644 util.h + +diff --git Makefile b/Makefile +index f7d3d2d..240669d 100644 +--- Makefile ++++ Makefile +@@ -25,7 +25,8 @@ VERSION=1.3.1 + DEFINES=-DPACKAGE=\"$(NAME)\" -DVERSION=\"$(VERSION)\" \ + -DPKGDATADIR=\"$(PREFIX)/share/slim\" -DSYSCONFDIR=\"$(CFGDIR)\" + +-OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o ++OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o \ ++ panel.o util.o + ifdef USE_PAM + OBJECTS+=PAM.o + endif +diff --git Makefile.freebsd b/Makefile.freebsd +index 3ff326e..c925a39 100644 +--- Makefile.freebsd ++++ Makefile.freebsd +@@ -24,7 +24,8 @@ VERSION=1.3.1 + DEFINES=-DPACKAGE=\"$(NAME)\" -DVERSION=\"$(VERSION)\" \ + -DPKGDATADIR=\"$(PREFIX)/share/slim\" -DSYSCONFDIR=\"$(CFGDIR)\" + +-OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o ++OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o \ ++ panel.o util.o + .ifdef USE_PAM + OBJECTS+=PAM.o + .endif +diff --git Makefile.netbsd b/Makefile.netbsd +index ad8bb8b..45f33e6 100644 +--- Makefile.netbsd ++++ Makefile.netbsd +@@ -24,7 +24,8 @@ VERSION=1.3.1 + DEFINES=-DPACKAGE=\"$(NAME)\" -DVERSION=\"$(VERSION)\" \ + -DPKGDATADIR=\"$(PREFIX)/share/slim\" -DSYSCONFDIR=\"$(CFGDIR)\" + +-OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o ++OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o \ ++ panel.o util.o + .ifdef USE_PAM + OBJECTS+=PAM.o + .endif +diff --git Makefile.openbsd b/Makefile.openbsd +index b1829f8..1205b84 100644 +--- Makefile.openbsd ++++ Makefile.openbsd +@@ -20,7 +20,8 @@ VERSION=1.3.1 + DEFINES=-DPACKAGE=\"$(NAME)\" -DVERSION=\"$(VERSION)\" \ + -DPKGDATADIR=\"$(PREFIX)/share/slim\" -DSYSCONFDIR=\"$(CFGDIR)\" + +-OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o ++OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o \ ++ util.o panel.o + + .SUFFIXES: .c.o .cpp.o + +diff --git app.cpp b/app.cpp +index 83ae947..04caaa1 100644 +--- app.cpp ++++ app.cpp +@@ -24,6 +24,7 @@ + #include <algorithm> + #include "app.h" + #include "numlock.h" ++#include "util.h" + + + #ifdef HAVE_SHADOW +@@ -1185,8 +1186,8 @@ void App::CreateServerAuth() { + 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) { +diff --git switchuser.cpp b/switchuser.cpp +index e72a8fc..ec298e1 100644 +--- switchuser.cpp ++++ switchuser.cpp +@@ -10,6 +10,7 @@ + */ + + #include "switchuser.h" ++#include "util.h" + + using namespace std; + +@@ -53,10 +54,10 @@ 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); + } +diff --git util.cpp b/util.cpp +new file mode 100644 +index 0000000..309ce4f +--- /dev/null ++++ util.cpp +@@ -0,0 +1,32 @@ ++/* SLiM - Simple Login Manager ++ Copyright (C) 2009 Eygene Ryabinkin <rea@codelabs.ru> ++ ++ This program is free software; you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation; either version 2 of the License, or ++ (at your option) any later version. ++*/ ++ ++#include <stdio.h> ++#include "util.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; ++} +diff --git util.h b/util.h +new file mode 100644 +index 0000000..8bd52be +--- /dev/null ++++ util.h +@@ -0,0 +1,19 @@ ++/* SLiM - Simple Login Manager ++ Copyright (C) 2009 Eygene Ryabinkin <rea@codelabs.ru> ++ ++ This program is free software; you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation; either version 2 of the License, or ++ (at your option) any later version. ++*/ ++#ifndef __UTIL_H__ ++#define __UTIL_H__ ++ ++#include <string> ++ ++namespace Util { ++ bool add_mcookie(const std::string &mcookie, const char *display, ++ const std::string &xauth_cmd, const std::string &authfile); ++}; ++ ++#endif /* __UTIL_H__ */ +-- +1.6.3.1 + diff --git a/x11/slim/files/patch-001-random-routines.diff b/x11/slim/files/patch-001-random-routines.diff new file mode 100644 index 0000000..4bdff31 --- /dev/null +++ b/x11/slim/files/patch-001-random-routines.diff @@ -0,0 +1,193 @@ +From 5beb217296e3074cadc5bcb3e40355f54ee705f0 Mon Sep 17 00:00:00 2001 +From: Eygene Ryabinkin <rea@shadow.codelabs.ru> +Date: Thu, 21 May 2009 11:56:27 +0400 +Subject: [PATCH] Create interface for random number generator and use it everywhere + +Don't use rand()/srand() at all -- they are very weak. Provide our +wrappers for random()/srandom() and make utility function that will +generate seed for srandom. + +Rework MIT magic cookie generation: consume 4 bytes of input in one +pass -- random() should produce values that are usable for this purpose. + +Signed-off-by: Eygene Ryabinkin <rea@shadow.codelabs.ru> +--- + app.cpp | 49 ++++++++++++++++++++++++++----------------------- + app.h | 2 ++ + util.cpp | 37 +++++++++++++++++++++++++++++++++++++ + util.h | 5 +++++ + 4 files changed, 70 insertions(+), 23 deletions(-) + +diff --git app.cpp b/app.cpp +index 04caaa1..0ac8c3a 100644 +--- app.cpp ++++ app.cpp +@@ -129,15 +129,18 @@ void User1Signal(int sig) { + + + #ifdef USE_PAM +-App::App(int argc, char** argv): +- pam(conv, static_cast<void*>(&LoginPanel)){ ++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) ++ : + #endif ++ mcookiesize(32) // Must be divisible by 4 ++{ + int tmp; + ServerPID = -1; + testing = false; +- mcookie = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; ++ mcookie = string(App::mcookiesize, 'a'); + daemonmode = false; + force_nodaemon = false; + firstlogin = true; +@@ -1128,13 +1131,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; +@@ -1161,27 +1164,27 @@ 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()); +diff --git app.h b/app.h +index 7b4bd10..9a44269 100644 +--- app.h ++++ app.h +@@ -101,6 +101,8 @@ private: + + std::string themeName; + std::string mcookie; ++ ++ const int mcookiesize; + }; + + +diff --git util.cpp b/util.cpp +index 309ce4f..5ed972f 100644 +--- util.cpp ++++ util.cpp +@@ -7,7 +7,13 @@ + (at your option) any later version. + */ + ++#include <sys/types.h> ++ + #include <stdio.h> ++#include <stdlib.h> ++#include <time.h> ++#include <unistd.h> ++ + #include "util.h" + + /* +@@ -30,3 +36,34 @@ bool Util::add_mcookie(const std::string &mcookie, const char *display, + 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); ++} +diff --git util.h b/util.h +index 8bd52be..b8d2993 100644 +--- util.h ++++ util.h +@@ -14,6 +14,11 @@ + 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); + }; + + #endif /* __UTIL_H__ */ +-- +1.6.3.1 + diff --git a/x11/slim/files/patch-Makefile.freebsd b/x11/slim/files/patch-Makefile.freebsd index 069550d..4d9af0c 100644 --- a/x11/slim/files/patch-Makefile.freebsd +++ b/x11/slim/files/patch-Makefile.freebsd @@ -1,5 +1,5 @@ ---- Makefile.freebsd.orig 2008-10-04 13:37:07.000000000 +0200 -+++ Makefile.freebsd 2008-10-04 13:40:07.000000000 +0200 +--- Makefile.freebsd.orig 2009-05-20 18:53:00.000000000 +0400 ++++ Makefile.freebsd 2009-05-20 18:53:39.000000000 +0400 @@ -3,18 +3,14 @@ # Edit the following section to adjust the options # to fit into your operating system / distribution @@ -27,15 +27,15 @@ DESTDIR= ####################################################### -@@ -24,10 +20,7 @@ - DEFINES=-DPACKAGE=\"$(NAME)\" -DVERSION=\"$(VERSION)\" \ +@@ -25,10 +21,7 @@ -DPKGDATADIR=\"$(PREFIX)/share/slim\" -DSYSCONFDIR=\"$(CFGDIR)\" --OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o + OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o \ +- panel.o util.o -.ifdef USE_PAM - OBJECTS+=PAM.o -.endif -+OBJECTS=jpeg.o png.o main.o image.o numlock.o cfg.o switchuser.o app.o panel.o PAM.o ++ panel.o util.o PAM.o all: slim -- 1.6.3.1 How-To-Repeat: [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306
Responsible Changed From-To: freebsd-ports-bugs->miwi miwi@ wants his PRs (via the GNATS Auto Assign Tool)
Maintainer of x11/slim, Please note that PR ports/134801 has just been submitted. If it contains a patch for an upgrade, an enhancement or a bug fix you agree on, reply to this email stating that you approve the patch and a committer will take care of it. The full text of the PR can be found at: http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/134801 -- Edwin Groothuis via the GNATS Auto Assign Tool edwin@FreeBSD.org
State Changed From-To: open->feedback Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
miwi 2009-05-30 19:18:58 UTC FreeBSD ports repository Modified files: x11/slim Makefile x11/slim/files patch-Makefile.freebsd Added files: x11/slim/files patch-000-fix-visible-mcookie patch-001-random-routines Log: - Fix local disclosure of X authority magic cookie PR: 134801 Submitted by: Eygene Ryabinkin <rea-fbsd@codelabs.ru> Approved by: maintainer timeout (security 8 days) Security: http://www.freebsd.org/ports/portaudit/80f13884-4d4c-11de-8811-0030843d3802.html Revision Changes Path 1.16 +1 -1 ports/x11/slim/Makefile 1.1 +192 -0 ports/x11/slim/files/patch-000-fix-visible-mcookie (new) 1.1 +193 -0 ports/x11/slim/files/patch-001-random-routines (new) 1.7 +6 -6 ports/x11/slim/files/patch-Makefile.freebsd _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
State Changed From-To: feedback->closed Committed. Thanks!