3 ms·
What's the fix for those code samples? Shellcheck currently gives Sample 1 a pass. I hope this is something it can be modified to catch.
by spiffytech 2y ago
What's the fix for those code samples?
Shellcheck currently gives Sample 1 a pass. I hope this is something it can be modified to catch.
- pizzalife 2y agoHonestly, the fix is to only allow alphanumeric input to shellscripts. Anything else invariably fails at some point.
- lmz 2y agoTaint checking as in Perl would be nice.
- webstrand 2y agoThere's now an issue for it https://github.com/koalaman/shellcheck/issues/3088 https://github.com/koalaman/shellcheck/issues/3088
- deleted 2y ago[deleted]
- usr1106 2y agoThe first function one is not particularly well-written, but harmless. The quoting of ${num} is completely useless. Inside [[ bash does not do any word splitting after variable expansion. Double quotes never prevent variable expansion. I am not sure what the author is talking about. Shellcheck is correct to not complain. I stopped reading there.
- woodruffw 2y ago> Double quotes never prevent variable expansion. I am not sure what the author is talking about. Shellcheck is correct to not complain. I stopped reading there. I think it would behoove you to read the rest of the post. The double quotes are not the operative part of example there; they're only there to demonstrate that the code execution doesn't come from splatting or word splitting. The actual code execution in Case #1 comes from the fact that bash (and other ksh descendants) run arithmetic evaluation on some strings in arithmetic contexts, regardless of their double or single quoting. That evaluation, in turn, can run arbitrary shell commands.
- usr1106 2y agoOk, need to read it again with more time. Myself I typically don't script in bash. Most of the extras like [[ are not needed, you can do everything in dash. Arrays are the only feature that comes to my mind where bash would be handy.
- casey2 2y agoI can only assume you were down-voted for calling bloat like arrays useful.
- usr1106 2y agoSo -eq triggers evaluation? Sounds like typical bash magic. I would use [ an the problem goes away. Showing -eq is not the best example, it can just be replaced by = and the problem goes away. But if you need -gt or similar there is no replacement. So one should stick to [. If I follow correctly the dangerous combination is [[ and arithmetic comparisons?
- woodruffw 2y ago`-eq` is for arithmetic comparison; `=` is for string comparison. They don't do the same thing, and it's unsound to uniformly replace either with the other. The dangerous thing here is that an undefined number of contexts exist where Bash treats strings as arithmetic expressions, which can contain arbitrary code despite not being quoted for expansion. `-eq` is just one example of that; others have linked other examples. (This is all for case #1. With case #2, `[` and `test` are also susceptible so long as their builtin variants are used.)