From b6b7e2ee2e6db849dc8509ce1c97e66dbb143635 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 4 Nov 2021 17:18:58 -0700 Subject: [PATCH] Add the missing '--' to shell invocations. This came up with POSIX recently. Doesn't seem like it matters since everyone's had this wrong for 40 years, but "meh" --- it's a trivial fix, and it's strictly correct even if nobody needs this, so let's just do it... (Geoff Clare pointed out that my app compat concern "what if someone's relying on this bug to pass flags to the shell?" isn't relevant because while you can indeed do that, you then can't pass a command!) Bug: https://austingroupbugs.net/view.php?id=1440 Test: treehugger Change-Id: I64f6440da55e2dc29d0136ee62007197d2f00d46 --- libc/bionic/system.cpp | 2 +- libc/stdio/stdio.cpp | 2 +- tests/stdlib_test.cpp | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/libc/bionic/system.cpp b/libc/bionic/system.cpp index 950f05c7d..93d7497c8 100644 --- a/libc/bionic/system.cpp +++ b/libc/bionic/system.cpp @@ -56,7 +56,7 @@ int system(const char* command) { if ((errno = posix_spawnattr_setsigmask64(&attributes, &sigchld_blocker.old_set_))) return -1; if ((errno = posix_spawnattr_setflags(&attributes, flags))) return -1; - const char* argv[] = { "sh", "-c", command, nullptr }; + const char* argv[] = {"sh", "-c", "--", command, nullptr}; pid_t child; if ((errno = posix_spawn(&child, __bionic_get_shell_path(), nullptr, &attributes, const_cast(argv), environ)) != 0) { diff --git a/libc/stdio/stdio.cpp b/libc/stdio/stdio.cpp index c429ff2c3..08df2ebbd 100644 --- a/libc/stdio/stdio.cpp +++ b/libc/stdio/stdio.cpp @@ -1219,7 +1219,7 @@ FILE* popen(const char* cmd, const char* mode) { if (dup2(fds[child], desired_child_fd) == -1) _exit(127); close(fds[child]); if (bidirectional) dup2(STDOUT_FILENO, STDIN_FILENO); - execl(__bionic_get_shell_path(), "sh", "-c", cmd, nullptr); + execl(__bionic_get_shell_path(), "sh", "-c", "--", cmd, nullptr); _exit(127); } diff --git a/tests/stdlib_test.cpp b/tests/stdlib_test.cpp index 6679480e4..6dbd74167 100644 --- a/tests/stdlib_test.cpp +++ b/tests/stdlib_test.cpp @@ -496,6 +496,30 @@ TEST(stdlib, system_NULL) { ASSERT_NE(0, system(nullptr)); } +// https://austingroupbugs.net/view.php?id=1440 +TEST(stdlib, system_minus) { + // Create a script with a name that starts with a '-'. + TemporaryDir td; + std::string script = std::string(td.path) + "/-minus"; + ASSERT_TRUE(android::base::WriteStringToFile("#!" BIN_DIR "sh\nexit 66\n", script)); + + // Set $PATH so we can find it. + setenv("PATH", td.path, 1); + // Make it executable so we can run it. + ASSERT_EQ(0, chmod(script.c_str(), 0555)); + + int status = system("-minus"); + EXPECT_TRUE(WIFEXITED(status)); + EXPECT_EQ(66, WEXITSTATUS(status)); + + // While we're here and have all the setup, let's test popen(3) too... + FILE* fp = popen("-minus", "r"); + ASSERT_TRUE(fp != nullptr); + status = pclose(fp); + EXPECT_TRUE(WIFEXITED(status)); + EXPECT_EQ(66, WEXITSTATUS(status)); +} + TEST(stdlib, atof) { ASSERT_DOUBLE_EQ(1.23, atof("1.23")); }