Bug 134801 - [vuxml][patch] x11/slim: document and patch CVE-2009-1756
Summary: [vuxml][patch] x11/slim: document and patch CVE-2009-1756
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Martin Wilke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-22 06:00 UTC by Eygene Ryabinkin
Modified: 2009-05-30 20:20 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eygene Ryabinkin 2009-05-22 06:00:13 UTC
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
Comment 1 Edwin Groothuis freebsd_committer 2009-05-22 06:00:25 UTC
Responsible Changed
From-To: freebsd-ports-bugs->miwi

miwi@ wants his PRs (via the GNATS Auto Assign Tool)
Comment 2 Edwin Groothuis freebsd_committer 2009-05-22 06:00:27 UTC
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
Comment 3 Edwin Groothuis freebsd_committer 2009-05-22 06:00:29 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Comment 4 dfilter service freebsd_committer 2009-05-30 20:19:07 UTC
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"
Comment 5 Martin Wilke freebsd_committer 2009-05-30 20:19:25 UTC
State Changed
From-To: feedback->closed

Committed. Thanks!