Fix insecure xauth secret

Patch from http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306

Signed-off-by: Nobuhiro Iwamatsu <[email protected]>

Makefile

@@ -7,7 +7,7 @@ CXX=/usr/bin/g++
 CC=/usr/bin/gcc
 CFLAGS=-Wall -I. -I/usr/include/freetype2 -I/usr/include/freetype2/config -I/usr/include/libpng12 -I/usr/include
 CXXFLAGS=$(CFLAGS)
-LDFLAGS=-lXft -lX11 -lfreetype -lXrender -lfontconfig -lpng12 -lz -lm -lcrypt -lXmu -lpng -ljpeg
+LDFLAGS=-lXft -lX11 -lfreetype -lXrender -lfontconfig -lpng12 -lz -lm -lcrypt -lXmu -lpng -ljpeg -lrt
 CUSTOM=-DHAVE_SHADOW
 ifdef USE_PAM
 LDFLAGS+= -lpam
@@ -25,7 +25,7 @@ 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

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

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

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
 

app.cpp

@@ -24,6 +24,7 @@
 #include <algorithm>
 #include "app.h"
 #include "numlock.h"
+#include "util.h"
 
 
 #ifdef HAVE_SHADOW
@@ -128,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;
@@ -1127,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;
@@ -1160,33 +1164,33 @@ 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) {

app.h

@@ -101,6 +101,8 @@ class App {
 
     std::string themeName;
     std::string mcookie;
+
+    const int mcookiesize;
 };
 
 

switchuser.cpp

@@ -11,6 +11,7 @@
 
 #include <cstdio>
 #include "switchuser.h"
+#include "util.h"
 
 using namespace std;
 
@@ -54,10 +55,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);
 }

util.cpp

@@ -0,0 +1,69 @@
+/* SLiM - Simple Login Manager
+   Copyright (C) 2009 Eygene Ryabinkin <[email protected]>
+
+   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 <sys/types.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.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;
+}
+
+/*
+ * 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);
+}

util.h

@@ -0,0 +1,24 @@
+/* SLiM - Simple Login Manager
+   Copyright (C) 2009 Eygene Ryabinkin <[email protected]>
+
+   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);
+
+	void srandom(unsigned long seed);
+	long random(void);
+
+	long makeseed(void);
+};
+
+#endif /* __UTIL_H__ */