Skip to content
wodry edited this page Aug 9, 2021 · 9 revisions

Injecting filenames is fragile and insecure. Use parameters.

Problematic code:

find . -name '*.mp3' -exec sh -c 'i="{}"; sox "$i" "${i%.mp3}.wav"' \;

Correct code:

find . -name '*.mp3' -exec sh -c 'i="$1"; sox "$i" "${i%.mp3}.wav"' _ {} \;

Rationale:

In the problematic example, the filename is passed by injecting it into a shell string. Any shell metacharacters in the filename will be interpreted as part of the script, and not as part of the filename. This can break the script and allow arbitrary code execution exploits.

In the correct example, the filename is passed as a parameter. It will be safely treated as literal text. The _ is a dummy string that becomes $0 in the script.

Exceptions:

None.

ShellCheck

Each individual ShellCheck warning has its own wiki page like SC1000. Use GitHub Wiki's "Pages" feature above to find a specific one, or see Checks.

Clone this wiki locally