Hacker News Reader

Comment by panzi | Hacker News Reader

Yeah, system() should definitely be deprecated and you should never use it if you write any new program. At least there is exec*() and posix_spawn() under POSIX. Under Windows there is no such thing and every program might parse the command line string differently. You can't naively write a generic posix_spawn() like interface for Windows, see this related Rust CVE: https://blog.rust-lang.org/2024/04/09/cve-2024-24576/ Why is it a CVE in Rust, but not in any other programming language? Did other language handle it better? Dunno, I just know that Rust has a big fat warning about this in their documentation (https://doc.rust-lang.org/std/process/struct.Command.html#me...), but e.g. Java doesn't (https://docs.oracle.com/javase/8/docs/api/java/lang/ProcessB...).

View on HN

The main reason system() exists is that people want to execute shell commands; some confused novice developers might mix it up with execl(), but this is not a major source of vulnerabilities. The major source of vulnerabilities is "oh yeah, I actually meant to execute shell".

So if you just take away the libcall, people will make their own version by just doing execl() of /bin/sh. If you want this to change, I think you have to ask why do people want to do this in the first place.

And the answer here is basically that because of the unix design philosophy, the shell is immensely useful. There are all these cool, small utilities and tricks you can use in lieu of writing a lot of extra code. On Windows, command-line conventions, filesystem quirks, and escaping gotchas are actually more numerous. It's just that there's almost nothing to call, so you get fewer bugs.

The most practical way to make this class of bugs go away is to make the unix shell less useful.

most calls to system() that I've seen could be replaced with exec without much difficulty. There's relatively few that actually need the shell functionality.

system() involves fork()ing, setting up signal handlers, exec()ing and wait()ing. You won't be replacing it with exec, most of the time you'll be reimplementing it for absolutely no reason.

Python has os.spawnl, os.spawnv, etc., which fork()s, wait4()s, etc., without involving a shell. This is much better; this is the library function you should be using instead of system() most of the time. Unfortunately I don't think glibc has an equivalent!

    strace -o tmp.spawnlp -ff python3 -c 'import os; os.spawnlp(os.P_WAIT, "true", "true")'

In parent:

    clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fdc03233310) = 225954
    wait4(225954, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 225954
    --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=225954, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---

In child:

    set_robust_list(0x7fdc03233320, 24)     = 0
    gettid()                                = 225954
    clock_gettime(CLOCK_MONOTONIC, {tv_sec=2458614, tv_nsec=322829153}) = 0
    clock_gettime(CLOCK_MONOTONIC, {tv_sec=2458614, tv_nsec=323030718}) = 0
    execve("/usr/local/bin/true", ["true"], 0x7ffdc5008458 /* 44 vars */) = -1 ENOENT (No such file or directory)
    execve("/usr/bin/true", ["true"], 0x7ffdc5008458 /* 44 vars */) = 0

Here, I think strace shows clone() rather than fork() because glibc's fork() is a library function that invokes clone(), rather than a real system call.

> Python has os.spawnl, os.spawnv, etc., which fork()s, wait4()s, etc., without involving a shell.

Good. How do you pipeline commands with these?

These functions can't do it. In Python you have to use the subprocess module if you want to pipeline commands without the bugs introduced by the shell. From https://docs.python.org/3.7/library/subprocess.html#replacin...:

    p1 = Popen(["dmesg"], stdout=PIPE)
    p2 = Popen(["grep", "hda"], stdin=p1.stdout, stdout=PIPE)
    p1.stdout.close()  # Allow p1 to receive a SIGPIPE if p2 exits.
    output = p2.communicate()[0]

Of course, now, nobody has an hda, and dmesg is root-only. A more modern example is in http://canonical.org/~kragen/sw/dev3/whereroot.py:

    p1 = subprocess.Popen(["df"], stdout=subprocess.PIPE)
    p2 = subprocess.Popen(["grep", "/$"], stdin=p1.stdout, stdout=subprocess.PIPE)
    p1.stdout.close()
    return p2.communicate()[0]

Note that the result here is a byte string, so if you want to print it out safely without the shell-like bugginess induced by Python's default character handling (what happens if the device name isn't valid UTF-8?), you have to do backflips with sys.stdout.buffer or UTF-8B.

Python got a lot of things wrong, and it gets worse all the time, but for now spawning subprocesses is one of the things it got right. Although, unlike IIRC Tcl, it doesn't raise an exception by default if one of the commands fails.

Apart from the semantics of the operations, you could of course desire a better notation for them. In Python you could maybe achieve something like

    (cmd(["df"]) | ["grep", "/$"]).output()

but that is secondary to being able to safely handle arguments containing spaces and pipes and whatnot.

Dunno, so much work to achieve so little. I'm even more inclined to stick with shell scripts now

The Bourne shell is definitely less work unless you want your code to correctly or reliably handle user input. Then it's more work.

Not in my experience. Any concrete examples off the top of your head, where it's more work than setting up pipes in python manually?

Fill in the blank to run a docker container which opens the file with user-provided path in (say) vim.

docker run --rm -it ...?

Now run a container doing the exact same thing ("docker-in-docker").

docker run --rm -it -v $DOCKER_HOST:/var/run/docker.sock ...?

> Fill in the blank to run a docker container which opens the file with user-provided path in (say) vim.

Never used docker before, but this seems to work:

    docker run --rm -it debian bash -c 'vim -- "$1"' _ "$user_provided_path"

Looks relatively safe to me, though it doesn't seem to work because debian:latest doesn't have vim in it (so I'm skeptical of your implicit claim of having tried it), and, if $user_provided_path is empty, it defaults to browsing the filesystem. But there are a lot of characters there that are specifically there to avoid footguns; without them, it would seem to work, but it would fail when $user_provided_path contained special characters.

The version I tested was

    docker run --rm -it debian bash -c 'apt update; apt install -y vim; vim -- "$1"' _ "$user_provided_path"

> your implicit claim of having tried it

I tried printing positional parameters, they looked fine. (And already uninstalled docker. What's the point of containerization if you need superuser privileges to use it?)

> if $user_provided_path is empty, it defaults to browsing the filesystem

That's what

    vim -- ""

does.

> But there are a lot of characters there that are specifically there to avoid footguns

What are those characters? --? That's not a lot

Also bash -c '' and "".

The Python code Kragen gave is more characters to type, but fewer footguns.

Shell scripts are much higher in footguns per character than most programming languages.

It is possible for a coder to understand bash so well that he never shoots his own foot off, but it requires more learning hours than the same feat in another language requires, and unless I've also put in the (many) learning hours, I have no way of knowing whether a shell script written by someone I don't know contains security vulnerabilities or fragility when dealing with unusual inputs that will surface in unpredictable circumstances.

The traditional Unix shell might be the most overrated tool on HN.

> The Python code Kragen gave is more characters to type, but fewer footguns.

What are the footguns in this?

    df | grep /$

There are none AFAIK.

Allow me to correct my previous comment: using Python's os.spawnlp or Popen to invoke Unix commands has fewer footguns than using the shell. (Kragen's code only addresses your question of how to set up a pipeline in Python.)

There is posix_spawn(). Some operating systems even implement that as a system call (not Linux). Implementing that as a system call has the advantage that spawning a new process from a process that has huge memory mapping is fast, because the memory mappings don't need to be copied (yes, I know the memory is copy on write, but the mappings themselves have to be correctly copied with the information needed for copy on write).

I don't understand what you are complaining about. I don't understand what the article is complaining about either.

exec* are not "better replacements" of the shell, they are just used for different use cases.

The whole article could be summarized to 3 bullet points:

1) Sanitize your inputs

2) If you want to execute a specific program, exec it after 1), no need for the shell

3) Allow the shell if there is no injection risk

The article spends a lot of time dancing around its central points rather than addressing them directly, but the basic problems with shell boil down to this:

There's two ways to think of "running a command:"

1. A list of strings containing an executable name (which may or may not be a complete path) and its arguments (think C's const char **argv).

2. A single string which is a space-separated list of arguments, with special characters in arguments (including spaces) requiring quoting to represent correctly.

Conversion between these two forms is non trivial. And the basic problem is that there's a lot of tools which incorrectly convert the former to the latter by just concatenating all of the arguments into a single string and inserting spaces. Part of the problem is that shell script itself makes doing the conversion difficult, but the end effect is that if you have to with commands with inputs that have special characters (including, but not limited to, spaces), you end up just going slowly insane trying to figure out how to get the quoting right to work around the broken tools.

In my experience, the world is so much easier if your own tools just break everything up into the list-of-strings model and you never to try to use an API that requires single-string model.

What GP is referring to is the fact that that solution doesn't work as well on Windows, because the OS's native idea of a command line isn't list-of-strings but rather a single-string, and how that single string is broken up into a list-of-strings is dependent on the application being invoked.

I think "non trivial" and "slowly going insane" parts only happen if you don't have right tools, or not using POSIX-compatable system.

In python you have "shlex.quote" and "shlex.join". In bash, you have "${env@Q}". I've found those to work wonderfully to me - and I did crazy things like quote arguments, embed into shell script, quote script again for ssh, and quote 3rd time to produce executable .sh file.

In other languages.. yeah, you are going to have bad time. Especially on Windows, where I'd just give up and move to WSL.

To be honest, I've never heard of Bash's @Q solution before today--I can't find it in https://tldp.org/LDP/abs/html/, which is my usual goto guide for "how do I do $ADVANCED_FEATURE in bash?"

To be fair that's missing a lot. I'm not sure how much is just showing its age and how much it never had. The actual bash manual is quite informative.

In particular, failure to mention `printf -v` is horrible. Not only is it better performing than creating a whole process for command substitution, it also avoids the nasty newline problem.

`printf -v` was added in Bash 3.1 (2005). I think revisions of ABS predates that; but ABS has certainly been updated since then (last in 2014), and has no excuse for not including it.

@Q was added in Bash 4.4 (2016), ABS was last updated in 2014.

I'd say: Don't use the shell if what you want to do is to execute another program.

You don't need to handle any quoting with exec*(). You still need to handle options, yes. But under Windows you always have to to handle the quoting yourself and it is more difficult than for the POSIX shell and it is program dependent. Without knowing what program is executed you can't know what quoting syntax you have to use and as such a standard library cannot write a generic interface to pass arguments to another process in a safe way under Windows.

I just felt it sounded like POSIX is particularly bad in that context, while in fact it is better than Windows here. Still, the system() function is a mistake. Use posix_spawn(). (Note: Do not use _spawn*() under Windows. That just concatenates the arguments with a space between and no quoting whatsoever.)

>Still, the system() function is a mistake. Use posix_spawn().

They are entirely different interfaces though. If you'd implemented system() using posix_spawn() it'd be just as bad as system()

Why would you implement system() at all?

parse commands from config file? command-line arguments for hooks?

https://news.ycombinator.com/item?id=44239036

I understand that it is convenient for running small snippets like that, but I don't really think it's worth the risk. And putting it into a config file is different, IMO. You don't get tempted to do some bad string interpolation there, because you can't, unless the config file format has support for that, but then I criticize that. If you need to pass things to such a snipped do it via environment variables or standard IO, not string interpolation.

If you say you don't make such mistakes: Yeah, but people do. People that write the code that runs on your system.

But if you want a command-line option for hook, what are the alternatives?

Force user to always create a wrapper script? that's just extra annoyance and if user is bad at quoting, they'll have the same problems with a script

Disable hooks at all? that's bad functionality regression

Ask for multiple arguments? this makes command-line parsing much more awkward.. I have not seen any good solutions for that.

(The only exception is writing a command wrapper that takes exactly 1 user command, like "timeout" or "xargs".. but those already using argument vector instead of parsing)

You define a config file format that supports only the minimal syntax required to specify a multi-argument command (e.g. spaces separate arguments, arguments with spaces in them may be quoted or use backslashes to escape them).

Then, you parse that out into a proper argument array and pass it to exec*/posix_spawn.

So instead of a well known (i.e. POSIX) quoting semantics and existing tool support, you want to introduce your own ad-hoc format? No thanks.

A correct parser for the syntax I described can be written in less than a 100 lines of code — even in C. It’s a strict subset of the shell command language defined by POSIX, and it’s sufficiently expressive as to support specifying any argument array unambiguously.

To correctly escape arbitrary shell syntax, not only do you need to handle the full POSIX syntax (which is quite complex) …

https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V...

… but you must also cover any bugs and undocumented/underspecified extensions implemented by the actual shell providing /bin/sh on every platform and platform version to which your code will be deployed.

That’s not just difficult — it’s impossible, and everyone that has tried has failed, repeatedly. Leading to security bugs, repeatedly.

https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9...

There’s a reason why we use parameterized queries instead of escaping to prevent SQL injection, and SQL syntax and parsing behavior is far more rigorously specified than the shell.

You don't need to care about the whole quoting spec. E.g. what you need to escape without quotes becomes irrelevant as soon as you consistently add quotes and what you can do with $ expressions is irrelevant if you simply escape all $. This is far from rocket science.

But it also doesn't matter how complex the standard is, it is a standard and one with much preexisting support. Your "standard" will end up being different for each application.

Decades of CVEs say otherwise.

Doing things the way you seem to prefer is just willful negligence at this point.

This is fear mongering. Neither your comments nor the links you posted shows how rolling your custom command interpreter syntax is in any way better than using the ones available on your target platforms.

Because I don't want to implement a shell???

If you want to run a shell script, run a shell script. I.e. a text file with the executable bit set and a shebang. If you want to generate a shell script on the fly to then run it, take a step back and think about what you're doing.

Or I can just use system() because it's there and not going anywhere

if every developer can follow best practice, we won't need Rust.

https://flatt.tech/research/posts/batbadbut-you-cant-securel...

Yeah, especially the thing about variable substitution is insane. How can you mess this up so thoroughly!? Appendix B is a nice overview. I'm amazed that Java doesn't even mention this in their documentation!

Java has a bunch of code which looks like it's trying to do the right kind of escaping for msvcrt vs cmd.exe:

https://github.com/openjdk/jdk/blob/jdk-26%2B1/src/java.base...

But i would be lying if i said i understood what was going on there. Some googling suggests this was added around 1.7, ie in the early 2010s.

But then, that Rust CVE seems to originate in this work, and this guy claims Java said "won't fix", which suggests it is vulnerable:

https://flatt.tech/research/posts/batbadbut-you-cant-securel...

But there's no link, and i can't find any discussion about it, so i don't know what the actual situation is.

Yeah, part of the problem is how Windows does variable substitution before the command line syntax is parsed, and at a glance I don't see any % in that file.

Reply on news.ycombinator.com