ShellCheck CWE mappings

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

ShellCheck CWE mappings

Steve Grubb
Hello,

This is the follow up I promised a few weeks ago about using CWE for shell
script flaws. The tool that was used is called ShellCheck. You can find the
source code here:

https://github.com/koalaman/shellcheck

And it has an extensive Wiki describing the error messages that it produces:

https://gist.github.com/nicerobot/53cee11ee0abbdc997661e65b348f375#file-_shellcheck-md

Recently a scan of over 3,900 open source packages containing a little over
5,400 shell scripts was done. The following are part of the top 15 shell
script problems:

                     Warning                                 How Many
   SC2039-undefined_posix_behavior   2566
     SC2046-prevent_word_splitting   2214
              SC2164-cd_might_fail         2037
   SC2166-use_separate_tests_style   1665
          SC2064-use_single_quotes       1577
   SC2155-return_value_masked_warn    983
              SC2148-unknown_shell        701
SC2121-prefer_assignment_not_set_warn    500
SC2048-prevent_whitespace_problem    325
           SC1083-brace_is_literal          281

What I'm trying to do is map the most common shell script errors to a CWE and
then make that warning number to CWE mapping available as a CSV file. The
aim is to confirm a mapping or pose the question of does this fit? I don't
know if anyone has tried fitting shell script to CWE, it seems like its not
a clean fit in some cases. And I also provide these examples in case any
existing content might be updated one day to show examples in other
languages.


SC2039
------
In POSIX sh, something is undefined. This issue arises because the programmer
started the program with #!/bin/sh  when they are clearly counting on
features in bash or dash or another shell. The fix is to use the right shell.

An example of echo flags:
    echo -n "$PIN" > "$PIN_SOURCE"

Example of 'local' is undefined:
    local PACKAGE="$3"

Example of  &> is undefined.
    if test -x $GDB2 && ! which $GDB &>/dev/null; then

I am wondering if CWE-475: 'Undefined Behavior for Input to API' would be
applicable? There's also CWE-688: 'Function Call With Incorrect Variable' but
this is not a function. One other candidate might be CWE-758: Reliance on
Undefined, Unspecified, or Implementation-Defined Behavior.


SC2046
------
This is described as "Quote this to prevent word splitting". When the results
of running a command are collected and have spaces in them, word splitting or
globbing can occur. If this is attacker controlled input, such as file names,
they may force running an unintended command.

rm `cat $FEDORA_LANGPACK_CONFIG` > /dev/null 2>&1
 kill -TERM $(cat /run/pand-${DEVICE}.pid)
[ -r /run/radvd/radvd.pid ] && kill -HUP $(cat /run/radvd/radvd.pid)

I am wondering if CWE-88: Improper Neutralization of Argument Delimiters in a
Command ('Argument Injection') would be correct?


SC2164
------
The cd command might fail. This could happen because the intended directory
doesn't exist or the user doesn't have permissions. The fix is to add || exit
after the cd so that the script doesn't perform operations in the wrong
directory.  Examples:

cd $workdir
cd ${OLDPATH}

I am wondering if CWE-252: "Unchecked Return Value" would be the correct
mapping?


SC2166
------
This is described as: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined
From POSIX: The XSI extensions specifying the -a and -o binary primaries and
the '(' and ')' operators have been marked obsolete. The fix is to convert
script from
if ! [ "$#" -ge 2 -a "$#" -le 3 ]; then
to
if ! [ "$#" -ge 2 ] && [-a "$#" -le 3 ]; then

Would this be CWE-758: "Reliance on Undefined, Unspecified, or Implementation-
Defined Behavior" or maybe CWE-477: "Use of Obsolete Function" ?


SC2064
------
This is described as Use single quotes, otherwise this expands now rather
than when signalled. This is only for trap commands which are signal handlers
in shell. The rationale is that the programmer probably intended the
expansion when the signal handler runs as opposed to when it is defined. This
can lead to incomplete cleanup of a directory

examples:
trap "rm -f $tmp_files" 0
trap "$RM $cwrappersource $cwrapper; exit $EXIT_FAILURE" 1 2 15

do_exit='(exit $ret); exit $ret'
trap "ret=129; $do_exit" 1

Would this be CWE-569: "Expression Issues"? Or maybe CWE-367: "Time-of-check
Time-of-use"? I don't think maps cleanly to either.


SC2155
------
This is described as Declare and assign separately to avoid masking return
values. The rationale is that the use of export will always return true. If
the code needed to test the return of the command or have a trap run, then
it will be masked. Examples:

export GOROOT="$(cd .. && pwd)"
local LOGFILE="$(mktemp)"

n both of these cases, neither programs check the return code and failure is
ignored. Would this map to CWE-571: "Expression is Always True" ?


SC2148
------
This warning is emitted whenever a shell script program does not start with
#!/bin/something. The fix is to specify a shell. The exception to this is when
the file is being sourced from another script which correctly specifies a
shell.

Examples for this are varied. But in all cases, line number 1 does not
specify a shell.

Would this be CWE-758: "Reliance on Undefined, Unspecified, or Implementation-
Defined Behavior" ?  It seems like a lot are winding up in this bucket.


SC2121
------
This is described as To assign a variable, use just 'var=value', not 'set'.
The rationale is that set does not assign a value. It's used to set shell
options. Examples:

set fnord $dstdir
set dummy --warnings none ${1+"$@"}

Would this be CWE-438: "Behavioral Problems". Or would CWE-456: "Missing
Initialization of a Variable" be the better fit?

SC2048
------
This is similar to SC2064. Except this one is concerned with uses of $*. The
rationale is that $* is used to reference all arguments. If one of them has
a whitespace or a character that causes globbing, then the code won't behave
as intended. Examples:

for name in $*; do
  one_user $name
done

main $*   <- Calls a main function

/usr/ucb/mail -f $*   <- Calls an external program

Would this fall into CWE-569: "Expression Issues" ? It doesn't seem like an
exact fit.


SC1083
------
This is described as "This {/} is literal". Something is wrong with the
expression which causes the braces to become literal rather than command
grouping or brace expansion.

Example 1:
local mnts=$(findmnt --mtab --source ${rbd_dev} --noheadings \
                                        | awk '{print $1'})
Example 2:
for i in {1 100} ; do

In the first one, the ' is on the wrong side of }. That makes the brace stand
alone instead of the brace ending awk code segment. In the second, the loop
executes twice, $i is '{1' and '100}'. The fix is to add '..' between the
numbers - {1..100}.

The best I could come up with is CWE-569: "Expression Issues". But it doesn't
seem like a good fit.