Skip to content

Commit 36ffe27

Browse files
committed
Use execve() to replace system()
- Direct Execution: execve() directly executes a program, bypassing the shell. This avoids vulnerabilities like shell injection, which can occur with system() if user input is not properly sanitized. - Controlled Environment: With execve(), you can explicitly specify the environment variables for the new process, providing better control over the execution context. - No Shell Overhead: execve() does not invoke a shell, reducing the risk of unintended behavior caused by shell features or configurations. - Predictable Behavior: execve() only executes the specified program, whereas system() relies on the shell, which may interpret commands differently based on the shell's configuration or environment.
1 parent c883faf commit 36ffe27

File tree

6 files changed

+91
-40
lines changed

6 files changed

+91
-40
lines changed

core/iwasm/compilation/aot_compiler.c

+23-19
Original file line numberDiff line numberDiff line change
@@ -4195,13 +4195,13 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
41954195
bh_print_time("Begin to emit object file");
41964196

41974197
if (comp_ctx->external_llc_compiler || comp_ctx->external_asm_compiler) {
4198-
char cmd[1024];
41994198
int ret;
42004199

42014200
if (comp_ctx->external_llc_compiler) {
4202-
const char *stack_usage_flag = "";
4203-
char bc_file_name[64];
4204-
char su_file_name[65]; /* See the comment below */
4201+
char *stack_usage_flag = "";
4202+
char bc_file_name[64] = { 0 };
4203+
char su_file_name[65] = { 0 };
4204+
char *argv[10] = { 0 };
42054205

42064206
if (comp_ctx->stack_usage_file != NULL) {
42074207
/*
@@ -4229,14 +4229,16 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
42294229
return false;
42304230
}
42314231

4232-
snprintf(cmd, sizeof(cmd), "%s%s %s -o %s %s",
4233-
comp_ctx->external_llc_compiler, stack_usage_flag,
4234-
comp_ctx->llc_compiler_flags ? comp_ctx->llc_compiler_flags
4235-
: "-O3 -c",
4236-
file_name, bc_file_name);
4237-
LOG_VERBOSE("invoking external LLC compiler:\n\t%s", cmd);
4232+
argv[0] = stack_usage_flag;
4233+
argv[1] = comp_ctx->llc_compiler_flags
4234+
? (char*)comp_ctx->llc_compiler_flags
4235+
: "-O3 -c";
4236+
argv[2] = "-o";
4237+
argv[3] = file_name;
4238+
argv[4] = bc_file_name;
4239+
argv[5] = NULL;
42384240

4239-
ret = bh_system(cmd);
4241+
ret = bh_system(comp_ctx->external_llc_compiler, argv, 6);
42404242
/* remove temp bitcode file */
42414243
unlink(bc_file_name);
42424244

@@ -4263,7 +4265,8 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
42634265
}
42644266
}
42654267
else if (comp_ctx->external_asm_compiler) {
4266-
char asm_file_name[64];
4268+
char asm_file_name[64] = { 0 };
4269+
char *argv[10] = { 0 };
42674270

42684271
if (!aot_generate_tempfile_name("wamrc-asm", "s", asm_file_name,
42694272
sizeof(asm_file_name))) {
@@ -4282,14 +4285,15 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
42824285
return false;
42834286
}
42844287

4285-
snprintf(cmd, sizeof(cmd), "%s %s -o %s %s",
4286-
comp_ctx->external_asm_compiler,
4287-
comp_ctx->asm_compiler_flags ? comp_ctx->asm_compiler_flags
4288-
: "-O3 -c",
4289-
file_name, asm_file_name);
4290-
LOG_VERBOSE("invoking external ASM compiler:\n\t%s", cmd);
4288+
argv[0] = comp_ctx->asm_compiler_flags
4289+
? (char*)comp_ctx->asm_compiler_flags
4290+
: "-O3 -c";
4291+
argv[1] = "-o";
4292+
argv[2] = file_name;
4293+
argv[3] = asm_file_name;
4294+
argv[4] = NULL;
42914295

4292-
ret = bh_system(cmd);
4296+
ret = bh_system(comp_ctx->external_asm_compiler, argv, 5);
42934297
/* remove temp assembly file */
42944298
unlink(asm_file_name);
42954299

core/iwasm/compilation/aot_emit_aot_file.c

+18-15
Original file line numberDiff line numberDiff line change
@@ -4339,18 +4339,23 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
43394339
else if (!strncmp(LLVMGetTargetName(target), "arc", 3)) {
43404340
/* Emit to assembly file instead for arc target
43414341
as it cannot emit to object file */
4342-
char file_name[] = "wasm-XXXXXX", buf[128];
4342+
char file_name[] = "wasm-XXXXXX";
4343+
char assembly_file_name[64] = { 0 };
4344+
char object_file_name[64] = { 0 };
43434345
int ret;
4346+
char *argv[] = { "-mcpu=arcem", "-o", object_file_name, "-c",
4347+
assembly_file_name, NULL };
43444348

43454349
if (!bh_mkstemp(file_name, sizeof(file_name))) {
43464350
aot_set_last_error("make temp file failed.");
43474351
goto fail;
43484352
}
43494353

4350-
snprintf(buf, sizeof(buf), "%s%s", file_name, ".s");
4354+
snprintf(assembly_file_name, sizeof(assembly_file_name) - 1, "%s.s",
4355+
file_name);
43514356
if (LLVMTargetMachineEmitToFile(comp_ctx->target_machine,
4352-
comp_ctx->module, buf, LLVMAssemblyFile,
4353-
&err)
4357+
comp_ctx->module, assembly_file_name,
4358+
LLVMAssemblyFile, &err)
43544359
!= 0) {
43554360
if (err) {
43564361
LLVMDisposeMessage(err);
@@ -4363,14 +4368,14 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
43634368
/* call arc gcc to compile assembly file to object file */
43644369
/* TODO: get arc gcc from environment variable firstly
43654370
and check whether the toolchain exists actually */
4366-
snprintf(buf, sizeof(buf), "%s%s%s%s%s%s",
4367-
"/opt/zephyr-sdk/arc-zephyr-elf/bin/arc-zephyr-elf-gcc ",
4368-
"-mcpu=arcem -o ", file_name, ".o -c ", file_name, ".s");
4371+
snprintf(object_file_name, sizeof(object_file_name) - 1, "%s.o",
4372+
file_name);
43694373
/* TODO: use try..catch to handle possible exceptions */
4370-
ret = bh_system(buf);
4374+
/* TODO: use ZEPHYR_SDK_INSTALL_DIR to construct the path */
4375+
ret = bh_system("/opt/zephyr-sdk/arc-zephyr-elf/bin/arc-zephyr-elf-gcc",
4376+
argv, 6);
43714377
/* remove temp assembly file */
4372-
snprintf(buf, sizeof(buf), "%s%s", file_name, ".s");
4373-
unlink(buf);
4378+
unlink(assembly_file_name);
43744379

43754380
if (ret != 0) {
43764381
aot_set_last_error("failed to compile asm file to obj file "
@@ -4379,12 +4384,10 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
43794384
}
43804385

43814386
/* create memory buffer from object file */
4382-
snprintf(buf, sizeof(buf), "%s%s", file_name, ".o");
4383-
ret = LLVMCreateMemoryBufferWithContentsOfFile(buf, &obj_data->mem_buf,
4384-
&err);
4387+
ret = LLVMCreateMemoryBufferWithContentsOfFile(
4388+
object_file_name, &obj_data->mem_buf, &err);
43854389
/* remove temp object file */
4386-
snprintf(buf, sizeof(buf), "%s%s", file_name, ".o");
4387-
unlink(buf);
4390+
unlink(object_file_name);
43884391

43894392
if (ret != 0) {
43904393
if (err) {

core/shared/utils/bh_common.c

+27-3
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,39 @@ wa_strdup(const char *s)
167167
}
168168

169169
#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
170+
/* need to make sure that The `argv[]` must be terminated by a NULL pointer. */
170171
int
171-
bh_system(const char *cmd)
172+
bh_system(const char *pathname, char *const argv[], int argc)
172173
{
173174
int ret;
175+
/* no environment variables */
176+
char *const envp[] = { NULL };
177+
178+
if (pathname == NULL) {
179+
return -1;
180+
}
181+
182+
if (argc > 0) {
183+
if (argv == NULL) {
184+
return -1;
185+
}
186+
187+
/* The `argv[]` must be terminated by a NULL pointer. */
188+
if (argv[argc - 1] != NULL) {
189+
return -1;
190+
}
191+
}
174192

175193
#if !(defined(_WIN32) || defined(_WIN32_))
176-
ret = system(cmd);
194+
ret = execve(pathname, argv, envp);
195+
#ifndef NDEBUG
196+
if (ret == -1) {
197+
LOG_WARNING("execute \"%s\" failed because of \"%s\"", pathname,
198+
strerror(errno));
199+
}
200+
#endif
177201
#else
178-
ret = _spawnlp(_P_WAIT, "cmd.exe", "/c", cmd, NULL);
202+
ret = _execve(pathname, argv, envp);
179203
#endif
180204

181205
return ret;

core/shared/utils/bh_common.h

+12-2
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,19 @@ char *
6767
wa_strdup(const char *s);
6868

6969
#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
70-
/* Executes a system command in bash/cmd.exe */
70+
/*
71+
* Executes a program referred to by cmd in bash/cmd.exe
72+
* Always be sure that argv[argc-1] == NULL
73+
*
74+
* @param pathname The program to execute. need to be absolute path.
75+
* @param argv The command line arguments.
76+
* @param argc The number of command line arguments.
77+
*
78+
* like to execute "ls -l /tmp":
79+
* bh_system("/bin/ls", (char *const []){ "-l", "/tmp", NULL }, 3);
80+
*/
7181
int
72-
bh_system(const char *cmd);
82+
bh_system(const char *pathname, char *const argv[], int argc);
7383

7484
/* Tests whether can create a temporary file with the given name */
7585
bool

tests/unit/shared-utils/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ set (unit_test_sources
2525

2626
add_executable (shared_utils_test ${unit_test_sources})
2727

28-
target_link_libraries (shared_utils_test gtest_main)
28+
target_link_libraries (shared_utils_test gtest_main ${LLVM_AVAILABLE_LIBS})
2929

3030
gtest_discover_tests(shared_utils_test)

tests/unit/shared-utils/bh_common_test.cc

+10
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,13 @@ TEST_F(bh_common_test_suite, b_memcpy_s)
9494
EXPECT_EQ(-1, b_memcpy_s(dest, sizeof(dest), nullptr, sizeof(STR_TEST)));
9595
EXPECT_EQ(-1, b_memcpy_s(dest, sizeof(dest), STR_TEST, sizeof(dest) + 1));
9696
}
97+
98+
TEST_F(bh_common_test_suite, bh_system)
99+
{
100+
#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
101+
char *const argv[] = { "-n", "\"hello\"", "world", "\n", nullptr };
102+
EXPECT_EQ(0, bh_system("/usr/bin/echo", argv, 5));
103+
#else
104+
GTEST_SKIP() << "bh_system() is not supported";
105+
#endif
106+
}

0 commit comments

Comments
 (0)