Hacker News new | ask | show | jobs
by pyre 4270 days ago
> Take a read of some of the earlier threads on HN about Shellshock, and you will find numerous people blaming Apache for not "escaping" the data it was putting in a shell variable.

It seems to me that this is the result of Cargo Cult Programming. People know that SQL strings, and user input need to be 'escaped,' so they just think "Obviously this needs to be escaped too! It's user input!" Yet they don't realize that they are trying to put a square peg in a round hole. They just know that pegs go through holes, so they keep banging away at it.

Also, it's always amazed me that there was never some sort of 'standard' way to shell-escape things, even though the shell has been around for ages. Why can't I generate a shell string in the same way that I generate a SQL string? E.g.:

  sprintf("mv %t %t", src, dest);
Where "%t" is a special token that shell-escapes the input (e.g. "My File Name.txt" => "My\ File\ Name.txt"). Instead it's something where people continue to use ad-hoc, incomplete, of 'implemented everywhere' solutions to this.

Note: I don't generate SQL strings with sprintf(), but it's a close approximation of:

  execute('select * from table where id = ?', id);
1 comments

SQL injection is not avoided by escaping arguments, but by never mixing the command and user supplied arguments in the first place. The equivalent to your example would be

  execlp("mv", "mv", src, dest, NULL);
which does not rely on the shell to try to untangle your arguments from a single string.

Edit: Fixed, thanks!

> SQL injection is not avoided by escaping arguments, but by never mixing the command and user supplied arguments in the first place.

People see:

  execute("select * from table where id = ?", id);
as for the most part like:

  execute(sprintf("select * from table where id = '%s'", escape(id)));
Where `escape()` is written by "smarter people" and makes sure that `id` isn't a string like this:

  0'; delete all from user;'
(e.g. turning it into `0''; delete all from user;''`). I realize that this isn't what actually happens, but the general idea is that you are sanitizing your inputs.
> Where `escape()` is written by "smarter people"

can we stop with this please? I'm sure it's not your intention and it's the way it's always phrased but it's casual contempt and we deserve to treat each other and be treated better.

"escape() is written by someone who spent the large amount of time analysing all the issues, testing, taking and incorporating feedback so the rest of us, who are both smart and competent, don't have to duplicate the work.

Similarly, no amount of escaping would protect you from Shellshock.

P.S. Doesn't execlp() require a NULL at the end of the parameter list?