Hacker News Reader

Comment by cbg0 | Hacker News Reader

It's actually like using unsanitized input in a lot of languages.

Specifically for PHP SQL injection, you can use PDO's prepared statements, and for shell commands you have https://www.php.net/manual/en/function.escapeshellcmd.php and https://www.php.net/manual/en/function.escapeshellarg.php

View on HN

If you're escaping shell tokens to reduce your arguments to a single executable command then you might as well not bother with forking to a shell in the first place and instead fork that executable directly since you're removing the one benefit forking to a shell has (effectively embedding a DSL). You'd get some small performance improvements in doing so too.

I think it makes sense to prefer this hypothetical API:

  subprocess.run_escaped("tac -- %s | grep 1 > %s", "foo.txt", "bar.txt")

over this:

  with open("bar.txt", "w") as outfile:
      tac = subprocess.Popen(["tac", "--", "foo.txt"], stdout=subprocess.PIPE)
      grep = subprocess.Popen(["grep", "1"], stdin=tac.stdout, stdout=outfile)
  tac.wait()
  grep.wait()

(In practice the shell grammar is so grotesque that I'd never trust it to be 100% correct. But the idea is nice.)

That doesn’t really solve the problem. Those shell tokens are still getting parsed and interpreted as special tokens only you’re now doing it in your host application rather than the Borne shell. And as you’ve pointed out, reimplementing shell syntax is non-trivial and highly error prone.

Plus if you’re going to those lengths then you’re better off embedding a Lua/Python/Perl/whatever interpreter in your application.

Reply on news.ycombinator.com