ports/134801: [vuxml][patch] x11/slim: document and patch CVE-2009-1756
Eygene Ryabinkin
rea-fbsd at codelabs.ru
Fri May 22 05:00:15 UTC 2009
>Number: 134801
>Category: ports
>Synopsis: [vuxml][patch] x11/slim: document and patch CVE-2009-1756
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: freebsd-ports-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Fri May 22 05:00:13 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator: Eygene Ryabinkin
>Release: FreeBSD 7.2-STABLE amd64
>Organization:
Code Labs
>Environment:
System: FreeBSD 7.2-STABLE amd64
>Description:
Nico Golde discovered insecure transfer of X magic cookie to the
.Xauthority file: [1]
>How-To-Repeat:
[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306
>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.
--- fix-visible-mcookie.diff begins here ---
>From a83e8761cdf36103748ee1df36c3e2eea8997faa Mon Sep 17 00:00:00 2001
From: Eygene Ryabinkin <rea-fbsd at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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
--- fix-visible-mcookie.diff ends here ---
The following VuXML entry should be evaluated and added:
--- vuln.xml begins here ---
<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 ---
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-ports-bugs
mailing list