From 3b30da2f2189451698628705cbaada4621416c99 Mon Sep 17 00:00:00 2001 From: koalaman Date: Sat, 23 May 2015 13:02:28 -0700 Subject: [PATCH] Created SC2156 (markdown) --- SC2156.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 SC2156.md diff --git a/SC2156.md b/SC2156.md new file mode 100644 index 0000000..efad4e9 --- /dev/null +++ b/SC2156.md @@ -0,0 +1,19 @@ +## 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. + +### Exceptions: + +None. \ No newline at end of file