Performing code review on shell scripts

• Check if the scripts are using absolute or relative paths. Using absolute paths wherever possible is recommended as a good practice.

• Check all input/output. This is probably the most common thing to look for even in shell scripts. It is recommended to take the input in double quotes eg. If ["$FOO" = "foo"].

• Check to make sure passwords, keys and other secrets are not present as environment variables or hardcoded in scripts. Search/grep for keywords. Some of the keywords to look for are given below.

• Check to see if any secret data is being passed in an external command's arguments. It is recommended to pass such data via a pipe or redirection instead.

• Check to make sure $PATH is always set carefully. PATH inherited from the caller should not be trusted, specially if the script is running as root. In fact, whenever an environment variable inherited from the caller is used, think about what could happen if the caller put something misleading in the variable, e.g., if the caller set $HOME to/etc.

• If one of the major usecases is "user logs in via SSH, and executes a script via sudo" think about using SSH command restriction instead. That way users don't get a shell on the target system at all, they can just execute that one command remotely.

• Check for Ownership, execution and permissions issues. Does the script contain info that others shouldn't be able to view? If so, make sure it's only readable by the owner. Check if the scripts have permissions to be executed by anybody or just by the owner of the script.

  • chmod 0700 is a good option • chmod u+x scriptname - executable bit is set for the user who owns the script only

• Be wary of symbolic links. These might need to be explored more.

• Temporary File Attack - look for secret data or anything sensitive being written to a fixed temp file. Writing anything sensitive to temporary files is never a good idea. Specially, when these files are generated with a fixed syntax i.e. test.tmp

• Input File Attack - see if contents from an input file (fixed path) is being sent over a different machine using "nc" or something like it. The input file can be manipulated and sensitive data can be made to sent to the attacker instead.

• Authentication Attacks - see if they are using standard variables like UID, USER and HOME to authenticate. These values can often be set and is not the right way to retrieve the user values.

References:

• http://tldp.org/LDP/abs/html/securityissues.html
• https://bbs.archlinux.org/viewtopic.php?pid=1235117
• http://stackoverflow.com/questions/8935162/shell-scripts-and-security
• http://searchenterpriselinux.techtarget.com/tip/Seven-tips-for-optimizing-shell-script-security
• https://code.google.com/p/shcov/

Keywords:

• password
• log
• chmod
• sudo
• UID
• USER
• HOME
• PATH
• Eval
• CLASSPATH