From 87b45bd0d18323898d5c09bb7cad7566e3267635 Mon Sep 17 00:00:00 2001
From: Mathias Gumz <akira at fluxbox dot org>
Date: Fri, 17 Sep 2010 23:43:24 +0200
Subject: bugfix: avoid naive use of 'putenv' by providing
 'FbTk::App::setenv()'

to quote from 'man putenv':

   The string pointed to by string becomes part of the environment,
   so altering the string changes the environment.

so, using putenv like

   {
      std::string foo("FOO=bar");
      putenv(foo.c_str());
   }

is wrong and leads to a potentially corrupted environment. valgrind
complaint correctly.

FbTk::App seems to be the appropriate place to hold '::seten()'
because it alters the environment of the application.
---
 src/FbCommands.cc | 39 +++++----------------------------------
 src/FbTk/App.cc   | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/FbTk/App.hh   |  5 +++++
 src/main.cc       |  3 +--
 4 files changed, 66 insertions(+), 36 deletions(-)

diff --git a/src/FbCommands.cc b/src/FbCommands.cc
index 8683711..7ff689e 100644
--- a/src/FbCommands.cc
+++ b/src/FbCommands.cc
@@ -142,8 +142,7 @@ int ExecuteCmd::run() {
     if (pid)
         return pid;
 
-    string displaystring("DISPLAY=");
-    displaystring += DisplayString(FbTk::App::instance()->display());
+    string display = DisplayString(FbTk::App::instance()->display());
     int screen_num = m_screen_num;
     if (screen_num < 0) {
         if (Fluxbox::instance()->mouseScreen() == 0)
@@ -158,12 +157,11 @@ int ExecuteCmd::run() {
     if (!shell)
         shell = "/bin/sh";
 
-    // remove last number of display and add screen num
-    displaystring.erase(displaystring.size()-1);
-    displaystring += FbTk::StringUtil::number2String(screen_num);
+    display.erase(display.size()-1);
+    display += FbTk::StringUtil::number2String(screen_num);
 
     setsid();
-    putenv(const_cast<char *>(displaystring.c_str()));
+    FbTk::App::setenv("DISPLAY", display.c_str());
     execl(shell, shell, "-c", m_cmd.c_str(), static_cast<void*>(NULL));
     exit(EXIT_SUCCESS);
 
@@ -197,34 +195,7 @@ ExportCmd::ExportCmd(const string& name, const string& value) :
 
 void ExportCmd::execute() {
 
-    // the setenv()-routine is not everywhere available and
-    // putenv() doesnt manage the strings in the environment
-    // and hence we have to do that on our own to avoid memleaking
-    static set<char*> stored;
-    char* newenv = new char[m_name.size() + m_value.size() + 2];
-    if (newenv) {
-
-        char* oldenv = getenv(m_name.c_str());
-
-        // oldenv points to the value .. we have to go back a bit
-        if (oldenv && stored.find(oldenv - (m_name.size() + 1)) != stored.end())
-            oldenv -= (m_name.size() + 1);
-        else
-            oldenv = NULL;
-
-        memset(newenv, 0, m_name.size() + m_value.size() + 2);
-        strcat(newenv, m_name.c_str());
-        strcat(newenv, "=");
-        strcat(newenv, m_value.c_str());
-
-        if (putenv(newenv) == 0) {
-            if (oldenv) {
-                stored.erase(oldenv);
-                delete[] oldenv;
-            }
-            stored.insert(newenv);
-        }
-    }
+    FbTk::App::instance()->setenv(m_name.c_str(), m_value.c_str());
 }
 
 REGISTER_COMMAND(exit, FbCommands::ExitFluxboxCmd, void);
diff --git a/src/FbTk/App.cc b/src/FbTk/App.cc
index 39cd36e..8157839 100644
--- a/src/FbTk/App.cc
+++ b/src/FbTk/App.cc
@@ -26,6 +26,21 @@
 
 #include "EventManager.hh"
 
+#ifdef HAVE_CSTRING
+  #include <cstring>
+#else
+  #include <string.h>
+#endif
+#ifdef HAVE_CSTDLIB
+  #include <cstdlib>
+#else
+  #include <stdlib.h>
+#endif
+
+
+#include <set>
+
+
 namespace FbTk {
 
 App *App::s_app = 0;
@@ -79,4 +94,44 @@ void App::end() {
     m_done = true; //end loop in App::eventLoop
 }
 
+bool App::setenv(const char* key, const char* value) {
+
+    if (!key || !*key)
+        return false;
+
+    static std::set<char*> stored;
+
+    const size_t key_size = strlen(key);
+    const size_t value_size = value ? strlen(value) : 0;
+
+    char* newenv = new char[key_size + value_size + 2];
+    if (newenv) {
+
+        char* oldenv = getenv(key);
+
+        // oldenv points to the value .. we have to go back a bit
+        if (oldenv && stored.find(oldenv - (key_size + 1)) != stored.end())
+            oldenv -= (key_size + 1);
+        else
+            oldenv = NULL;
+
+        memset(newenv, 0, key_size + value_size + 2);
+        strcat(newenv, key);
+        strcat(newenv, "=");
+        if (value_size > 0)
+            strcat(newenv, value);
+
+        if (putenv(newenv) == 0) {
+            if (oldenv) {
+                stored.erase(oldenv);
+                delete[] oldenv;
+            }
+            stored.insert(newenv);
+        }
+        return true;
+    }
+
+    return false;
+}
+
 } // end namespace FbTk
diff --git a/src/FbTk/App.hh b/src/FbTk/App.hh
index d878661..b83dd15 100644
--- a/src/FbTk/App.hh
+++ b/src/FbTk/App.hh
@@ -53,6 +53,11 @@ public:
     /// forces an end to event loop
     void end();
     bool done() const { return m_done; }
+
+    // the setenv()-routine is not everywhere available and
+    // putenv() doesnt manage the strings in the environment
+    // and hence we have to do that on our own to avoid memleaking
+    static bool setenv(const char* key, const char* value);
 private:
     static App *s_app;
     bool m_done;
diff --git a/src/main.cc b/src/main.cc
index 300826e..28302d4 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -219,8 +219,7 @@ static void parseOptions(int argc, char** argv, Options& opts) {
             }
 
             opts.session_display = argv[i];
-            string display_env = "DISPLAY=" + opts.session_display;
-            if (putenv(const_cast<char *>(display_env.c_str()))) {
+            if (!FbTk::App::setenv("DISPLAY", argv[i])) {
                 cerr<<_FB_CONSOLETEXT(main, WarnDisplayEnv,
                                 "warning: couldn't set environment variable 'DISPLAY'",
                               "")<<endl;
-- 
cgit v0.11.2