9 SC2156

Abubakar Nur Khalil edited this page 2021-08-21 10:12:05 +01:00

Table of Contents

• Injecting filenames is fragile and insecure. Use parameters.
• Problematic code:
• Correct code:
• Rationale:
• Exceptions:

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"' shell {} \;

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. Note that when using the shell command with -c, the first parameter to the shell command (in the example "shell") becomes $0 in the shell command's environment, where it is used e.g. in shell error messages (you can set it to an arbitrary value, but it makes sense to set it to the shell's name). You should not use the first parameter to the shell command as a data processing parameter because you cannot, for example, access $0 via $* in the shell command (because $* starts with $1), and as previously mentioned, $0 is used in the shell command's error messages, which would be confusing.

Exceptions:

None.

ShellCheck

• Installation

  • CentOS6
  • More Installation Guides

• Usage

  • Directive
  • Ignore

• Integrating and extending

  • Misc. contributions
  • Azure Pipelines
  • Buildkite
  • CircleCI
  • DevGuide
  • Integration
  • JUnit
  • Phabricator
  • TravisCI
  • GitLab CI
  • Drone
  • GitHub Actions

• Severity

• Template

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.