8

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?

  • 4
    Depending on how you can invoke the shell, you can also pass var as an argument. E.g. calling sh with arguments -c, 'cd "$1"; echo > create_a_file_here', 'sh', var works and does not need any changes to var. The 'sh' argument is passed as $0. – ipsec Jan 9 '18 at 19:38
  • 1
    What programming language? Are you using POSIX sh, or creating your own programming language with a similar syntax but which expands var instead of requiring you to write cd "$var"? Or is this bash with shopt -s cdable_vars? Oh, I think you mean that some other program forks a shell to run these commands. So just quote var, but make sure it doesn't include a quote character itself... – Peter Cordes Jan 10 '18 at 3:40
  • @PeterCordes If you're talking about Bash, a double quoted variable which contains a double quote is fine. e.g. s='"'; echo "$s" prints ". – wjandrea Jan 10 '18 at 7:00
  • @WJAndrea: yes, but there's no "trump card" quote that can't be defeated when constructing a variable assignment from untrusted input. Oh, solution: do var=untrusted string in the parent program, so var is an environment variable that's already set when invoking sh. Then you just need to quote it every time you expand it, which is possible to do reliably. Ah, I see that idea is already part of Stéphane's answer >.< – Peter Cordes Jan 10 '18 at 9:03
9

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=...

12

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.

  • 1
    And make sure whatever you use to match against [a-zA-Z0-9] doesn't include things like à whose encoding could also be misinterpreted by some shells (like bash) in some locales. – Stéphane Chazelas Jan 9 '18 at 15:05
  • @StéphaneChazelas (out of interest :) are they (and just under those affected locales?) – Wilf Jan 9 '18 at 23:34
10

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?

  • Thanks for your answer, but unfortunately i have to use the workaround with the bash commands. I tested quoting the variable - as muru suggested - and it seems to work! – ES___ Jan 9 '18 at 10:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.