Feature request: warn when echo prints text containing an untrusted variable

[austin987]

For new checks and feature suggestions

• [X] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
• [X] I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

Here's a snippet or screenshot that shows the problem:

#!/bin/sh
my_var="-e"
echo "$my_var"

Here's what shellcheck currently says:

$ shellcheck myscript
No issues detected!

Here's what I wanted or expected to see:

$ shellcheck myscript
error: passing a variable to echo as the first argument can cause unexpected behavior if variable starts with '/' or '-'. Use 'printf '%s\n' "$my_var"' instead.

For more details, see https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo/65819#65819.

This recently caused a nasty issue at my workplace. Having shellcheck catch this would be very handy.

[andlrc]

You shouldn't use echo at all, use printf instead:

printf '%s\n' "$my_var"

[austin987]

On Dec 19, 2016 12:57 AM, "Andreas Louv" [email protected] wrote:

You shouldn't use echo at all, use printf instead:

printf '%s\n' "$my_var"

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/koalaman/shellcheck/issues/796#issuecomment-267894879, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcXCG4YxIeBikXCpEnlVysy7OJDkGsSks5rJirXgaJpZM4LLZdD .

Beyond the first argument bring a variable? Based on what?

Not saying you're wrong, but you've offered no reasoning at all..

[andlrc]

There is a good write up over at unix/SE on the reasons to prefer printf: https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo

[austin987]

I've read that, it's included in the original report. AFAICT, the only real danger is when passing a variable as the first argument that could have a leading '-' or '/'. You're asserting that printf is always preferred, I'm asking what other reasons you have.

I suppose expansion characters could be another reason, though.

[austin987]

Actually, this should probably be at least two different checks. /bin/sh: echo with any arguments isn't portable echo with expansion characters (\n, \t, \, etc.) may have unexpected results (#771)

All shells: echo with variable as first arg may have unexpected results

potentially some others. But I think warning a user over echoing plain text, e.g. echo "hello world" is overkill.

I can take a try adding some warnings for these (I've never used Haskell, but working from prior commits it should be doable). If I don't get it done this week though, it may be a while because of holidays :).

@koalaman, do you have any thoughts? I don't want to spend time on it if you're opposed.

[koalaman]

Sounds useful.

There are cases like separator='======'; echo "$separator" where the warning is provably not useful (the codebase makes this kind of analysis difficult atm), and some edge cases like echo "${var//-/}" and echo ${var:+foo}.

Warning about literal backslash sequences in sh is more straight forward and should definitely be put in place.

[mohd-akram]

This remains an issue. I think this is in a similar class of errors as unquoted variables. There are cases when quoting is provably not required but shellcheck cannot recognize that, yet there's no real harm in quoting. I think the same approach should be taken with printf - err on the side of recommending it.