aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMathias Gumz <akira at fluxbox dot org>2010-09-17 21:43:24 (GMT)
committerMathias Gumz <akira at fluxbox dot org>2010-09-17 21:43:24 (GMT)
commit87b45bd0d18323898d5c09bb7cad7566e3267635 (patch)
treef5570d524399e3dab443dde9419af86c3874f416
parentf3ad09c4ce70cc58871eeac49d640f213f3cd16f (diff)
downloadfluxbox_pavel-87b45bd0d18323898d5c09bb7cad7566e3267635.zip
fluxbox_pavel-87b45bd0d18323898d5c09bb7cad7566e3267635.tar.bz2
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.
-rw-r--r--src/FbCommands.cc39
-rw-r--r--src/FbTk/App.cc55
-rw-r--r--src/FbTk/App.hh5
-rw-r--r--src/main.cc3
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() {
142 if (pid) 142 if (pid)
143 return pid; 143 return pid;
144 144
145 string displaystring("DISPLAY="); 145 string display = DisplayString(FbTk::App::instance()->display());
146 displaystring += DisplayString(FbTk::App::instance()->display());
147 int screen_num = m_screen_num; 146 int screen_num = m_screen_num;
148 if (screen_num < 0) { 147 if (screen_num < 0) {
149 if (Fluxbox::instance()->mouseScreen() == 0) 148 if (Fluxbox::instance()->mouseScreen() == 0)
@@ -158,12 +157,11 @@ int ExecuteCmd::run() {
158 if (!shell) 157 if (!shell)
159 shell = "/bin/sh"; 158 shell = "/bin/sh";
160 159
161 // remove last number of display and add screen num 160 display.erase(display.size()-1);
162 displaystring.erase(displaystring.size()-1); 161 display += FbTk::StringUtil::number2String(screen_num);
163 displaystring += FbTk::StringUtil::number2String(screen_num);
164 162
165 setsid(); 163 setsid();
166 putenv(const_cast<char *>(displaystring.c_str())); 164 FbTk::App::setenv("DISPLAY", display.c_str());
167 execl(shell, shell, "-c", m_cmd.c_str(), static_cast<void*>(NULL)); 165 execl(shell, shell, "-c", m_cmd.c_str(), static_cast<void*>(NULL));
168 exit(EXIT_SUCCESS); 166 exit(EXIT_SUCCESS);
169 167
@@ -197,34 +195,7 @@ ExportCmd::ExportCmd(const string& name, const string& value) :
197 195
198void ExportCmd::execute() { 196void ExportCmd::execute() {
199 197
200 // the setenv()-routine is not everywhere available and 198 FbTk::App::instance()->setenv(m_name.c_str(), m_value.c_str());
201 // putenv() doesnt manage the strings in the environment
202 // and hence we have to do that on our own to avoid memleaking
203 static set<char*> stored;
204 char* newenv = new char[m_name.size() + m_value.size() + 2];
205 if (newenv) {
206
207 char* oldenv = getenv(m_name.c_str());
208
209 // oldenv points to the value .. we have to go back a bit
210 if (oldenv && stored.find(oldenv - (m_name.size() + 1)) != stored.end())
211 oldenv -= (m_name.size() + 1);
212 else
213 oldenv = NULL;
214
215 memset(newenv, 0, m_name.size() + m_value.size() + 2);
216 strcat(newenv, m_name.c_str());
217 strcat(newenv, "=");
218 strcat(newenv, m_value.c_str());
219
220 if (putenv(newenv) == 0) {
221 if (oldenv) {
222 stored.erase(oldenv);
223 delete[] oldenv;
224 }
225 stored.insert(newenv);
226 }
227 }
228} 199}
229 200
230REGISTER_COMMAND(exit, FbCommands::ExitFluxboxCmd, void); 201REGISTER_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 @@
26 26
27#include "EventManager.hh" 27#include "EventManager.hh"
28 28
29#ifdef HAVE_CSTRING
30 #include <cstring>
31#else
32 #include <string.h>
33#endif
34#ifdef HAVE_CSTDLIB
35 #include <cstdlib>
36#else
37 #include <stdlib.h>
38#endif
39
40
41#include <set>
42
43
29namespace FbTk { 44namespace FbTk {
30 45
31App *App::s_app = 0; 46App *App::s_app = 0;
@@ -79,4 +94,44 @@ void App::end() {
79 m_done = true; //end loop in App::eventLoop 94 m_done = true; //end loop in App::eventLoop
80} 95}
81 96
97bool App::setenv(const char* key, const char* value) {
98
99 if (!key || !*key)
100 return false;
101
102 static std::set<char*> stored;
103
104 const size_t key_size = strlen(key);
105 const size_t value_size = value ? strlen(value) : 0;
106
107 char* newenv = new char[key_size + value_size + 2];
108 if (newenv) {
109
110 char* oldenv = getenv(key);
111
112 // oldenv points to the value .. we have to go back a bit
113 if (oldenv && stored.find(oldenv - (key_size + 1)) != stored.end())
114 oldenv -= (key_size + 1);
115 else
116 oldenv = NULL;
117
118 memset(newenv, 0, key_size + value_size + 2);
119 strcat(newenv, key);
120 strcat(newenv, "=");
121 if (value_size > 0)
122 strcat(newenv, value);
123
124 if (putenv(newenv) == 0) {
125 if (oldenv) {
126 stored.erase(oldenv);
127 delete[] oldenv;
128 }
129 stored.insert(newenv);
130 }
131 return true;
132 }
133
134 return false;
135}
136
82} // end namespace FbTk 137} // 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:
53 /// forces an end to event loop 53 /// forces an end to event loop
54 void end(); 54 void end();
55 bool done() const { return m_done; } 55 bool done() const { return m_done; }
56
57 // the setenv()-routine is not everywhere available and
58 // putenv() doesnt manage the strings in the environment
59 // and hence we have to do that on our own to avoid memleaking
60 static bool setenv(const char* key, const char* value);
56private: 61private:
57 static App *s_app; 62 static App *s_app;
58 bool m_done; 63 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) {
219 } 219 }
220 220
221 opts.session_display = argv[i]; 221 opts.session_display = argv[i];
222 string display_env = "DISPLAY=" + opts.session_display; 222 if (!FbTk::App::setenv("DISPLAY", argv[i])) {
223 if (putenv(const_cast<char *>(display_env.c_str()))) {
224 cerr<<_FB_CONSOLETEXT(main, WarnDisplayEnv, 223 cerr<<_FB_CONSOLETEXT(main, WarnDisplayEnv,
225 "warning: couldn't set environment variable 'DISPLAY'", 224 "warning: couldn't set environment variable 'DISPLAY'",
226 "")<<endl; 225 "")<<endl;