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