Skip to content

Commit bd7e22d

Browse files
an-taoan-tao
authored
Fix: replace system() with execvp() in SharedLibManager (#2465)
1 parent 9a2d672 commit bd7e22d

2 files changed

Lines changed: 231 additions & 27 deletions

File tree

‎fix.patch‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
diff --git a/lib/src/SharedLibManager.cc b/lib/src/SharedLibManager.cc
2+
index bf67b4f2..7a035a48 100644
3+
--- a/lib/src/SharedLibManager.cc
4+
+++ b/lib/src/SharedLibManager.cc
5+
@@ -17,10 +17,49 @@
6+
#include <dirent.h>
7+
#include <dlfcn.h>
8+
#include <fstream>
9+
+#include <sstream>
10+
#include <sys/types.h>
11+
+#include <sys/wait.h>
12+
#include <trantor/utils/Logger.h>
13+
#include <unistd.h>
14+
15+
+// Safe exec helper: runs a program with explicit argv, no shell involved.
16+
+// Returns the exit status, or -1 on fork/exec failure.
17+
+static int safeExec(const std::vector<std::string> &args)
18+
+{
19+
+ if (args.empty())
20+
+ return -1;
21+
+
22+
+ std::vector<char *> argv;
23+
+ argv.reserve(args.size() + 1);
24+
+ for (auto &a : args)
25+
+ argv.push_back(const_cast<char *>(a.c_str()));
26+
+ argv.push_back(nullptr);
27+
+
28+
+ pid_t pid = fork();
29+
+ if (pid == -1)
30+
+ {
31+
+ perror("fork");
32+
+ return -1;
33+
+ }
34+
+ if (pid == 0)
35+
+ {
36+
+ // Child: replace image with the target program.
37+
+ execvp(argv[0], argv.data());
38+
+ // execvp only returns on error.
39+
+ perror("execvp");
40+
+ _exit(127);
41+
+ }
42+
+ // Parent: wait for child.
43+
+ int status = 0;
44+
+ if (waitpid(pid, &status, 0) == -1)
45+
+ {
46+
+ perror("waitpid");
47+
+ return -1;
48+
+ }
49+
+ return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
50+
+}
51+
+
52+
static void forEachFileIn(
53+
const std::string &path,
54+
const std::function<void(const std::string &, const struct stat &)> &cb)
55+
@@ -153,20 +192,18 @@ void SharedLibManager::managerLibs()
56+
else
57+
{
58+
// generate source code and compile it.
59+
- std::string cmd = "drogon_ctl create view ";
60+
- if (!outputPath_.empty())
61+
- {
62+
- cmd.append(filename).append(" -o ").append(
63+
- outputPath_);
64+
- }
65+
- else
66+
- {
67+
- cmd.append(filename).append(" -o ").append(
68+
- libPath);
69+
- }
70+
+ const std::string &outDir =
71+
+ !outputPath_.empty() ? outputPath_ : libPath;
72+
+ std::vector<std::string> genArgs = {"drogon_ctl",
73+
+ "create",
74+
+ "view",
75+
+ filename,
76+
+ "-o",
77+
+ outDir};
78+
srcFile.append(".cc");
79+
- LOG_TRACE << cmd;
80+
- auto r = system(cmd.c_str());
81+
+ LOG_TRACE << "drogon_ctl create view " << filename
82+
+ << " -o " << outDir;
83+
+ auto r = safeExec(genArgs);
84+
// TODO: handle r
85+
(void)(r);
86+
dlStat.handle =
87+
@@ -203,24 +240,45 @@ void *SharedLibManager::compileAndLoadLib(const std::string &sourceFile,
88+
void *oldHld)
89+
{
90+
LOG_TRACE << "src:" << sourceFile;
91+
- std::string cmd = COMPILER_COMMAND;
92+
- cmd.append(" ")
93+
- .append(sourceFile)
94+
- .append(" ")
95+
- .append(COMPILATION_FLAGS)
96+
- .append(" ")
97+
- .append(INCLUDING_DIRS);
98+
- if (std::string(COMPILER_ID).find("Clang") != std::string::npos)
99+
- cmd.append(" -shared -fPIC -undefined dynamic_lookup -o ");
100+
- else
101+
- cmd.append(" -shared -fPIC --no-gnu-unique -o ");
102+
auto pos = sourceFile.rfind('.');
103+
auto soFile = sourceFile.substr(0, pos);
104+
soFile.append(".so");
105+
- cmd.append(soFile);
106+
- LOG_TRACE << cmd;
107+
108+
- if (system(cmd.c_str()) == 0)
109+
+ // Build argv without invoking a shell so that metacharacters in
110+
+ // sourceFile or soFile cannot be interpreted by /bin/sh.
111+
+ std::vector<std::string> compileArgs;
112+
+ compileArgs.push_back(COMPILER_COMMAND);
113+
+
114+
+ // COMPILATION_FLAGS and INCLUDING_DIRS are baked in at build time from
115+
+ // trusted CMake variables; split them on whitespace into separate tokens.
116+
+ auto splitIntoArgs = [&](const std::string &s) {
117+
+ std::istringstream iss(s);
118+
+ std::string token;
119+
+ while (iss >> token)
120+
+ compileArgs.push_back(token);
121+
+ };
122+
+ compileArgs.push_back(sourceFile);
123+
+ splitIntoArgs(COMPILATION_FLAGS);
124+
+ splitIntoArgs(INCLUDING_DIRS);
125+
+ if (std::string(COMPILER_ID).find("Clang") != std::string::npos)
126+
+ {
127+
+ compileArgs.push_back("-shared");
128+
+ compileArgs.push_back("-fPIC");
129+
+ compileArgs.push_back("-undefined");
130+
+ compileArgs.push_back("dynamic_lookup");
131+
+ }
132+
+ else
133+
+ {
134+
+ compileArgs.push_back("-shared");
135+
+ compileArgs.push_back("-fPIC");
136+
+ compileArgs.push_back("--no-gnu-unique");
137+
+ }
138+
+ compileArgs.push_back("-o");
139+
+ compileArgs.push_back(soFile);
140+
+
141+
+ LOG_TRACE << COMPILER_COMMAND << " " << sourceFile << " ... -o " << soFile;
142+
+
143+
+ if (safeExec(compileArgs) == 0)
144+
{
145+
LOG_TRACE << "Compiled successfully:" << soFile;
146+
return loadLib(soFile, oldHld);

‎lib/src/SharedLibManager.cc‎

Lines changed: 85 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,49 @@
1717
#include <dirent.h>
1818
#include <dlfcn.h>
1919
#include <fstream>
20+
#include <sstream>
2021
#include <sys/types.h>
22+
#include <sys/wait.h>
2123
#include <trantor/utils/Logger.h>
2224
#include <unistd.h>
2325

26+
// Safe exec helper: runs a program with explicit argv, no shell involved.
27+
// Returns the exit status, or -1 on fork/exec failure.
28+
static int safeExec(const std::vector<std::string> &args)
29+
{
30+
if (args.empty())
31+
return -1;
32+
33+
std::vector<char *> argv;
34+
argv.reserve(args.size() + 1);
35+
for (auto &a : args)
36+
argv.push_back(const_cast<char *>(a.c_str()));
37+
argv.push_back(nullptr);
38+
39+
pid_t pid = fork();
40+
if (pid == -1)
41+
{
42+
perror("fork");
43+
return -1;
44+
}
45+
if (pid == 0)
46+
{
47+
// Child: replace image with the target program.
48+
execvp(argv[0], argv.data());
49+
// execvp only returns on error.
50+
perror("execvp");
51+
_exit(127);
52+
}
53+
// Parent: wait for child.
54+
int status = 0;
55+
if (waitpid(pid, &status, 0) == -1)
56+
{
57+
perror("waitpid");
58+
return -1;
59+
}
60+
return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
61+
}
62+
2463
static void forEachFileIn(
2564
const std::string &path,
2665
const std::function<void(const std::string &, const struct stat &)> &cb)
@@ -153,20 +192,18 @@ void SharedLibManager::managerLibs()
153192
else
154193
{
155194
// generate source code and compile it.
156-
std::string cmd = "drogon_ctl create view ";
157-
if (!outputPath_.empty())
158-
{
159-
cmd.append(filename).append(" -o ").append(
160-
outputPath_);
161-
}
162-
else
163-
{
164-
cmd.append(filename).append(" -o ").append(
165-
libPath);
166-
}
195+
const std::string &outDir =
196+
!outputPath_.empty() ? outputPath_ : libPath;
197+
std::vector<std::string> genArgs = {"drogon_ctl",
198+
"create",
199+
"view",
200+
filename,
201+
"-o",
202+
outDir};
167203
srcFile.append(".cc");
168-
LOG_TRACE << cmd;
169-
auto r = system(cmd.c_str());
204+
LOG_TRACE << "drogon_ctl create view " << filename
205+
<< " -o " << outDir;
206+
auto r = safeExec(genArgs);
170207
// TODO: handle r
171208
(void)(r);
172209
dlStat.handle =
@@ -203,24 +240,45 @@ void *SharedLibManager::compileAndLoadLib(const std::string &sourceFile,
203240
void *oldHld)
204241
{
205242
LOG_TRACE << "src:" << sourceFile;
206-
std::string cmd = COMPILER_COMMAND;
207-
cmd.append(" ")
208-
.append(sourceFile)
209-
.append(" ")
210-
.append(COMPILATION_FLAGS)
211-
.append(" ")
212-
.append(INCLUDING_DIRS);
213-
if (std::string(COMPILER_ID).find("Clang") != std::string::npos)
214-
cmd.append(" -shared -fPIC -undefined dynamic_lookup -o ");
215-
else
216-
cmd.append(" -shared -fPIC --no-gnu-unique -o ");
217243
auto pos = sourceFile.rfind('.');
218244
auto soFile = sourceFile.substr(0, pos);
219245
soFile.append(".so");
220-
cmd.append(soFile);
221-
LOG_TRACE << cmd;
222246

223-
if (system(cmd.c_str()) == 0)
247+
// Build argv without invoking a shell so that metacharacters in
248+
// sourceFile or soFile cannot be interpreted by /bin/sh.
249+
std::vector<std::string> compileArgs;
250+
compileArgs.push_back(COMPILER_COMMAND);
251+
252+
// COMPILATION_FLAGS and INCLUDING_DIRS are baked in at build time from
253+
// trusted CMake variables; split them on whitespace into separate tokens.
254+
auto splitIntoArgs = [&](const std::string &s) {
255+
std::istringstream iss(s);
256+
std::string token;
257+
while (iss >> token)
258+
compileArgs.push_back(token);
259+
};
260+
compileArgs.push_back(sourceFile);
261+
splitIntoArgs(COMPILATION_FLAGS);
262+
splitIntoArgs(INCLUDING_DIRS);
263+
if (std::string(COMPILER_ID).find("Clang") != std::string::npos)
264+
{
265+
compileArgs.push_back("-shared");
266+
compileArgs.push_back("-fPIC");
267+
compileArgs.push_back("-undefined");
268+
compileArgs.push_back("dynamic_lookup");
269+
}
270+
else
271+
{
272+
compileArgs.push_back("-shared");
273+
compileArgs.push_back("-fPIC");
274+
compileArgs.push_back("--no-gnu-unique");
275+
}
276+
compileArgs.push_back("-o");
277+
compileArgs.push_back(soFile);
278+
279+
LOG_TRACE << COMPILER_COMMAND << " " << sourceFile << " ... -o " << soFile;
280+
281+
if (safeExec(compileArgs) == 0)
224282
{
225283
LOG_TRACE << "Compiled successfully:" << soFile;
226284
return loadLib(soFile, oldHld);

0 commit comments

Comments
 (0)