Hacker News Reader

Comment by o11c | Hacker News Reader

This is woefully misguided. Half the time passing it to the shell is explicitly a feature, e.g. `popen("gzip > foo.gz")`. If you have user input you should always sanitize it regardless of API.

But `ssh` does deserve all the shame. It's a pity the real problems are hard to find in an article full of nonsense.

Note also that if you're using a deficient shell that supports neither `printf %q` nor `${var@Q}` it's still easy to do quoting in `sed`. GNU `./configure` scripts do this internally, including special-casing to only quote the right side of `--arg=value`.

View on HN

> Note also that if you're using a deficient shell that supports neither `printf %q` nor `${var@Q}` it's still easy to do quoting in `sed`. GNU `./configure` scripts do this internally, including special-casing to only quote the right side of `--arg=value`.

With the assumption that:

1. The person knows to do this weird thing 2. They do it consistently every time 3. They never forget

Also not sure how to use those solutions for the popen() example you provided.

The correct way is:

    subprocess.run([
       "gzip",
       "-c",
       "—-to-stdout",
       user_input
    ], stdout=open("foo.gz"))

And now I don’t have to worry about any of these weird things

You need to ensure that user_input doesn't start with `-`. You can do that by forcing an absolute path. Some programs accept `--` as a marker that any arguments after that are non-options.

No need for an absolute path, just a './' prefix.

Unless the path was already absolute.

Yes, but running it through a function that turns it into an absolute path is a way to ensure that. A way. An easy way.

The idea is you have some 3rd-party app, which might accept a parameter and pass it to popen as-is, something like "--data-handler=command"

In this case, current "popen" semantics - a single shell command - works pretty well. You can pass it a custom process:

    --data-handler="scripts/handle_data.py"

or a shell fragment:

    --data-handler="gzip -c > last_data.gz"

or even mini-shell script:

    --data-handler "jq .contents | mail -s 'New data incoming' data-notify"

this is where the "shell command" arguments really shine - and note you cannot simulate all of this functionality with command vector.

Yes, in this specific use case you need a shell.

But that’s the same as saying you technically need SQL injection so that `psql -c 'command'` can work

> you cannot simulate all of this with command vector

Uhh, yes we can just call a shell:

    subprocess.run(["bash", "-c", data_handler])

As a bonus this way we get control of which shell is being used and I find it is more explicit so I prefer it

> subprocess.run(["bash",

Not the same thing, this is vulnerable to $PATH interception. You can hardcode the path to bash to avoid that but there's no guarantee that it'll always be there. system() on the other hand is guaranteed to run the operating system's command interpreter.

Yes the user controls the path and in the example provided they could just call whatever command they want anyway

This doesn’t give the attacker any access that they wouldn’t have.

Also you can just clobber the “PATH” variable if it is so inclined.

> system() on the other hand is guaranteed to run the operating system’s command intepreter

Yeah that just it is means less predictable.

Please show me python programs which support the pattern shown by the parent post and which actually work when running under powershell. (Or Oil shell or any other non-POSIX shell)

Aside: `/bin/sh` is guaranteed to exist by POSIX

> `/bin/sh` is guaranteed to exist by POSIX

Quite the contrary

> Applications should note that the standard PATH to the shell cannot be assumed to be either /bin/sh or /usr/bin/sh, and should be determined by interrogation of the PATH returned by getconf PATH,ensuring that the returned pathname is an absolute pathname and not a shell built-in.

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

    subprocess.run(["bash", "-c", "--", data_handler])

The very thing TFA complains about.

Do you think using `psql -c "SELECT 1"` is actually doing sql injection?

Because yeah if your program provides “invoking the shell as a feature” then it sure as fuck needs to invoke the shell. I was just replying to this far-fetched example.

By the way, I think it is still better to do this than calling system because if I read “run([bash” I know the developer meant to do this explicitly. If I read “system()” then I’m probably gonna assume they were just lazy and probably didn’t even know about the extra shell being invoked. (I also said this in my previous comment, please read before replying)

If you're sanitizing, you're losing. You need to either have a) a watertight escaping process or b) a format that doesn't mix the code and data in the first place (notably, shell lacks either).

> article full of nonsense

Pls elaborate. Seems like a decent list of shell gotchas to me.

It is kind of a journey in an annoying way. Like, do we really need to know all the stuff about: this man page says to sanitize input, this one doesn’t, blah blah blah.

Or, let’s just look at an excerpt, here’s the section “proper solution:”

I’ve emphasized the actionable advice.

> The proper solution would be dropping that broken tool immediately, securely erasing it from your hard-drive, then running and screaming that tool's name out-loud in shame... (Something akin to Game of Throne's walk of atonement...)

Joke

> I'm not kidding... This kind of broken tools are the cause of many stupid bugs, ranging from the funny ups-rm-with-spaces (i.e. rm -Rf / some folder with spaces /some-file), to serious security issues like the formerly mentioned shellshock...

Joke/contentless stakes raising.

> So, you say someone holds you at gun point, thus you must use that tool? Check if the broken tool doesn't have a flag that disables calling sh -c, and instead properly executes the given command and arguments directly via execve(2). (For example watch has the -x flag as mentioned.)

Here it is, the paragraph that has something!

> Alternatively, given that most likely the tool in question is an open-source project written by someone in his spare time, perhaps open a feature request describing the issue, and if possible contribute with a patch that solves it.

This doesn’t seem practically actionable, at least in the short term—most projects might ignore your patch, or maybe it will take multiple years to get pushed out to distros.

> Still no luck? Make some popcorn and prepare for the latest block-buster "convoluted solutions for simple problems in UNIX town"...

Dramatic buildup/joke.

The original post asserted the article is nonsense; you're trying to justify that by saying you don't like the author's writing style. Two separate things...

The article is mostly correct, although it makes some weird claims (e.g., the Shellshock bug had nothing to do with the class of bugs the article is complaining about - it was a vulnerability in the shell itself). It definitely has a "newcomer hates things without understanding why they are the way they are" vibe, but you actually need that every now and then. The old-timers tend to say "it was originally done this way for a reason and if you're experienced enough, you know how to deal with it", but what made sense 30-40 years ago might not make much sense today.

I dunno, "mostly" normally means some large fraction, maybe 50% or 90% depending on the person.. Given that executing commands by itself is neither a bug nor a security vulnerability (those only occur from bad/lack-of quoting), the majority of the article is wrong.

Reply on news.ycombinator.com