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
This commit is contained in:
Elliott Hughes 2021-11-04 17:18:58 -07:00
parent 7d3f322e64
commit b6b7e2ee2e
3 changed files with 26 additions and 2 deletions

View File

@ -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<char**>(argv), environ)) != 0) {

View File

@ -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);
}

View File

@ -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"));
}