Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

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.


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.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: