openbsd-ports/x11/slim/patches/patch-app_cpp

$OpenBSD: patch-app_cpp,v 1.2 2009/11/05 19:05:12 landry Exp $
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.
fixes CVE-2009-1756
--- app.cpp.orig	Fri Sep 26 02:54:15 2008
+++ app.cpp	Wed Oct 28 19:31:08 2009
@@ -32,6 +32,62 @@
 
 using namespace std;
 
+/* Code taken from http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=529306 */
+/* From: Eygene Ryabinkin <rea@shadow.codelabs.ru> */
+#include <time.h>
+#include <stdlib.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);
+}
+
 #ifdef USE_PAM
 #include <string>
 
@@ -104,7 +160,8 @@ extern App* LoginApp;
 
 void CatchSignal(int sig) {
     cerr << APPNAME << ": unexpected signal " << sig << endl;
-    LoginApp->StopServer();
+    if (LoginApp->serverStarted)
+        LoginApp->StopServer();
     LoginApp->RemoveLock();
     exit(ERR_EXIT);
 }
@@ -131,12 +188,13 @@ void User1Signal(int sig) {
 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) : mcookiesize(32) {
 #endif
     int tmp;
     ServerPID = -1;
+    serverStarted = false;
     testing = false;
-    mcookie = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+    mcookie = string(App::mcookiesize, 'a');
     daemonmode = false;
     force_nodaemon = false;
     firstlogin = true;
@@ -856,6 +914,8 @@ int App::StartServer() {
     char* args = new char[argOption.length()+2]; // NULL plus vt
     strcpy(args, argOption.c_str());
 
+    serverStarted = false;
+
     int argc = 1;
     int pos = 0;
     bool hasVtSet = false;
@@ -935,7 +995,7 @@ int App::StartServer() {
     }
     
     delete args;
-
+    serverStarted = true;
     return ServerPID;
 }
 
@@ -1127,13 +1187,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;
@@ -1159,34 +1219,32 @@ 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) {