Hacker News new | ask | show | jobs
by bonkabonka 1653 days ago
The original example MUST be corrected for the same reason that folks who post naive code snippets adding SQL strings together with user input must be corrected.

It is not a matter of taste and it is not a matter of metric versus imperial screwdrivers. Someone will copy this code and it will end up being an attack vector where it will have consequences.

I imagine you're rolling your eyes and have flipped the bozo bit but please bear with me.

Think of the teachable moment this presents! The author of the original piece goes back and annotates their original answer along the lines of, "you might solve it this way but there are some gotchas with it - let me show you what could go wrong."

As an industry we absolutely need to circle back with improvements so that those who come after us can build on a more solid foundation.

1 comments

Genuinely curious, what's so bad about the original code that it adds an attack vector?
To guard against malicious filenames I use `find ... -print0 | xargs -r0...` since posix disallows null bytes (and forward slashes) in filenames. The `-r` flag on xargs means it doesn't execute its command if find matches nothing.

So filenames can contain valid commands delimited with semi-colons that, if not quoted properly, can be unexpectedly run alongside your intended pipeline (say if you're doing the usual and unsafe "for csv in *.csv; do cat $csv; done").

I wish I could've laid my hands on the excellent HN thread from some years back that opened my eyes to this vector, but I'm hopeful someone else will mention it so I can add it to my bash notes file. :P