Hacker News Reader

Comment by fuzzbang | Hacker News Reader

You should clean the data from the user before passing it to the shell. There is a trivial remote command execution vulnerability in the URL ("echo 'GET /;$(cat /etc/passwd)'|nc ..."). I assume there are more.

View on HN

There are likely many MANY more :-) I suppose I should cook up a sanitising method. Fortunately, stackoverflow to the rescue! http://stackoverflow.com/questions/89609/in-a-bash-script-ho...

We have basic filtering now :-)

You shouldn't have to sanitize if you quote your variables properly. If general, the rules for "$foo" are "parse first, expand afterwards" while those for $foo are "parse, expand, parse again", so

  foo="hello    world"
  echo $foo # prints "hello world" [2 arguments]
  echo "$foo" # prints "hello    world" [1 argument]

Note that you can't trivially inject a command this way, though you can inject arbitrary arguments:

  foo="; ls"
  echo $foo # prints "; ls"

I would recommend never using test ([) with more than three arguments. Once you start doing -a and -o, then people can inject confusing values for variables, making it impossible to parse:

  foo='!'
  bar='2'
  [ "$foo" = 1 -a "$bar" = 2 ] # -bash: [: too many arguments

Since test runs a subprocess, it can't tell the difference between data and arguments. Instead, you should do something like this:

  foo='!'
  bar='2'
  [ "$foo" = 1 ] && [ "$bar" = 2 ]

(This also has the advantage of being way more legible.)

You can also solve that problem with double square brackets:

  [[ "$foo" = 1 && "$bar" = 2 ]]

Really, we should be using double square brackets. We're not aiming for POSIX compliance and double square solves a lot of problems.

Reply on news.ycombinator.com