Protection of shell command with string variable

Question

Within a programming language, i execute a simple shell command

cd var; echo > create_a_file_here

with var being a variable that contains a string of (hopefully) a directory to the place where i want to create the file "create_a_file_here". Now if someone sees this line of code, it is possible to exploit it by assigning for instance:

var = "; rm -rf /"

Things can get pretty ugly. One way to avoid the above case would be maybe to search the string in var for some special characters like ';' before executing the shell command, but i doubt this covers all possibly exploits.

Does anyone know a good way to ensure that "cd var" only changes a directory and nothing else?

Answer 1

Simple solution: Don't call the shell from your program. At all.

Your example here is trivial, changing the directory and creating files should be easy in whatever programming language. But even if you do need to run an external command, there's usually no need to do it through the shell.

So, e.g. in Python, instead of running os.system("somecmd " + somearg), use subprocess.run(["somecmd", somearg]). In C, instead of system(), use fork() and exec() (or find a library that does it).

If you need to use the shell, quote the command line arguments, or pass them through the environment, as in Stéphane's answer. Also, if you find yourself worrying about special character, the correct solution is to not try to filter out (blacklist) potentially dangerous characters, but to only keep characters known to be safe (whitelist).

Only allow the characters whose functions you do know, that way there's less risk of missing something. The end result might be that you only decide to allow [a-zA-Z0-9_], but that just might be enough to get the job done. You may also want to check that your locale and toolset don't include accented letters like ä and ö in that. They probably aren't considered special by any shell, but again, it's better to be sure if they pass or not.

Answer 2

If I understand correctly, var is a variable in your programming language.

And in your programming language, you're asking a shell to interpret a string that is the concatenation of "cd ", the content of that variable and "; echo > create_a_file_here".

If then, yes, if the content of var is not tightly controlled, it's a command injection vulnerability.

You could try and properly quote the content of the variable¹ in the syntax of the shell so it is guaranteed to be passed as a single argument to the cd builtin.

Another approach would be to pass the content of that variable another way. An obvious way would be to pass that in an environment variable. For instance, in C:

char *var =  "; rm -rf /";
setenv("DIR", var, 1);
system("CDPATH= cd -P -- \"$DIR\" && echo something > create_a_file_here");

This time, the code that you ask the shell to interpret is fixed, we still need to write it properly in the shell's syntax (here assumed to be a POSIX-compliant shell):

• shell variable expansion must be quoted to prevent split+glob
• you need -P for cd to do a simple chdir()
• you need -- to mark the end of options to avoid problems with var starting with - (or + in some shells)
• We set CDPATH to the empty string in case it was is in the environment
• We only run the echo command if cd was successful.

There is (at least) one remaining problems: if var is -, it doesn't chdir into the directory called - but to the previous directory (as stored in $OLDPWD) and OLDPWD=- CDPATH= cd -P -- "$DIR" is not guaranteed to work around it. So you'd need something like:

system(
  "case $DIR in\n"
  " (-) CDPATH= cd -P ./-;;\n"
  " (*) CDPATH= cd -P -- \"$DIR\";;\n"
  "esac && ....");

---

¹ Note that just doing a system(concat("cd \"", var, "\"; echo...")); is not the way to go, you'd be just moving the problem.

For instance, a var = "$(rm -rf /)" would still be a problem.

The only reliable way to quote text for Bourne-like shells is to use single quotes and also take care of the single quotes that may occur in the string. For instance, turn a char *var = "ab'cd" to char *escaped_var = "'ab'\\''cd'". That is, replace all ' to '\'' and wrap the whole thing inside '...'.

That still assumes that that quoted string is not used within backticks, and you'd still need the --, -P, &&, CDPATH=...

Answer 3

Within a programming language, there should be better ways to do things than executing shell commands. For example, replacing cd var with your programming language's equivalent of chdir (var); should ensure that any trickery with the value of var only results in a "Directory not found" error rather than unintended and possibly malicious actions.

Also, you can use absolute paths rather than changing directories. Just concatenate the directory name, slash and the filename you wish to use.

In C, I might do something like:

char filepath[PATH_MAX];  /* alternative constant: MAXPATHLEN */

/* Join directory name in var and the filename, guarding against exceeding PATH_MAX */
snprintf (filepath, PATH_MAX, "%s/%s", var, "create_a_file_here");

/* create an empty file/truncate an existing one */
fclose (fopen (filepath, "w") );

Surely your programming language can do something similar?