8

Given the script below, how can I ensure that the argument only contains a valid filename within /home/charlesingalls/ and not a path (../home/carolineingalls/) or wildcard, etc?

I only want the script to be able to delete a single file from the given hard-coded directory. This script will run as a privileged user.

#!/bin/bash

rm -f /home/charlesingalls/"$1"
  • 2
    If you don't want to support "foo/bar", just check that it doesn't contain /. Wildcards aren't interpreted inside quotes. – Random832 Jul 19 '16 at 5:44
  • 3
    If you're deleting a file, don't use -r with rm. rm -r is for recursive deletion of a directory and all files and directories beneath it. It is only useful when deleting directories. More generally, don't cargo-cult. i.e. don't just copy things that look useful into your command line or script without understanding what they do or how they work. The plane gods that bring the magic cargo can get angry and delete all your files. – cas Jul 19 '16 at 13:47
  • Good point regarding the -r flag - I do understand its use, just wasn't thinking clearly. – Aaron Cicali Jul 19 '16 at 15:36
7

If you only want to delete a file in /home/charlesingalls (and not a file in a subdirectory) then it's easy: just check that the argument doesn't contain a /.

case "$1" in
  */*) echo 1>&2 "Refusing to remove a file in another directory"; exit 2;;
  *) rm -f /home/charlesingalls/"$1";;
esac

This runs rm even if the argument is . or .. or empty, but in that case rm will harmlessly fail to delete a directory.

Wildcards are not relevant here since no wildcard expansion is performed.

This is safe even in the presence of symbolic links: if the file is a symbolic link, the symlink (which is in /home/charlesingalls) gets removed, and the target of that link is not affected.

Note that this assumes that /home/charlesingalls cannot be moved or changed. That should be ok if the directory is hard-coded in the script, but if it's determined from variables then the determination might no longer be valid by the time the rm command runs.

Based on the additional information that the argument is a virtual host name, you should do whitelisting rather than blacklisting: check that the name is a sensible virtual host name, rather than just banning slashes. I check that the name starts with a lowercase letter or digit and that it does not contain characters other than lowercase letters, digits, dots and dashes.

LC_CTYPE=C LC_COLLATE=C
case "$1" in
  *[!-.0-9a-z]*|[!0-9a-z]*) echo >&2 "Invalid host name"; exit 2;;
  *) rm -f /home/charlesingalls/"$1";;
esac
  • In this case, the files are web server configuration files that have been generated by a previous process. They all have the same level of importance (each one constitutes a virtual host). However, the directory they exist in is adjacent to similar directories, and they all exist under a directory that is owned by the web server. This particular script is meant to allow deletion of virtual host configs under a certain directory only. – Aaron Cicali Jul 19 '16 at 16:48
  • does this approach make sense to you provided that use case? I appreciate your experience and will admit there are definitely times I've "brought a bazooka to a gunfight" to automate something in linux. – Aaron Cicali Jul 19 '16 at 18:07
  • @AaronCicali Why can the script delete any of the host configs and not just one that belongs to the entity that made the original request? Why wasn't the virtual host name validated first (then it wouldn't contain any special characters)? – Gilles Jul 19 '16 at 18:16
  • The entity that makes the original request is a GUI that does indeed have permission to remove any of the virtual hosts in that folder. The virtual host name is validated first. It comes from a list of previously created virtual hosts (domain names). I believe it is the job of this script to ensure that it is as secure as it can be without relying on the security of another part of the application. Namely, that it can only delete files from within this specific directory. It will also need to do some additional cleanup work. – Aaron Cicali Jul 19 '16 at 18:22
  • @AaronCicali Ok, as an additional sanity check this makes sense. In this case you should whitelist: only accept names that look like plausible host names. If you don't allow subdomains, you might even forbid . – Gilles Jul 19 '16 at 18:42
10

This answer assumes that $1 is allowed to include subdirectories. If you are interested in the simpler case where $1 should be a simple directory name, then see one of the other answers.


Wildcards are not expanded when in double-quotes. Since $1 is in double-quotes, wildcards are not a problem.

Both ../ and symlinks can obscure the real location of a file. Shown below are tests to determine if the file is really, not just seemingly, under the path we want.

Newer systems: using realpath

As for finding out if the file is really if the file is really under /home/charlesingalls/ or not, you can use realpath:

realpath --relative-base=/home/charlesingalls/ "/home/charlesingalls/$1"  | grep -q '^/' && exit 1

The above runs exit 1 if the file specified by $1 is anywhere other than under the directory /home/charlesingalls/. realpath canonicalizes the whole path, eliminating both symlinks and ../.

realpath is part of GNU coreutils and should be available on any Linux system.

realpath requires GNU coreutils 8.15 (Jan 2012) or better.

Examples

To demonstrate how realpath follows ../ to determine the real location of a file (for examples, the -q option to grep is omitted so that the actual output of grep is visible):

$ touch /tmp/test
$ realpath --relative-base=$HOME "$HOME/../../tmp/test" | grep '^/' && echo FAIL
/tmp/test
FAIL

To demonstrate how it follows symlinks:

$ ln -s /tmp/test ~/test
$ realpath --relative-base=$HOME "$HOME/test" | grep '^/' && echo FAIL
/tmp/test
FAIL

Older systems: using readlink -e

readlink is also capable of cononicalizing a path, following both symlinks and ../:

readlink -e "$HOME/test" | grep -q "^$HOME" || exit 1

Using the same example files:

$ readlink -e "$HOME/../../tmp/test" | grep "$HOME" || echo FAIL
FAIL
$ readlink -e "$HOME/test" | grep "^$HOME" || echo FAIL
FAIL

In addition to being available on older GNU systems, versions of readlink are available on BSD.

  • My Ubuntu 14.04's coreutils does not have realpath – heemayl Jul 19 '16 at 2:45
  • 1
    This seems to be more of what I'm looking for, but unfortunately my Centos 6.5 server does not have realpath. I'm not in a position to install it. Googling turns up mentions of "readlink -f" superceding realpath, but I've yet to make it work. – Aaron Cicali Jul 19 '16 at 2:58
  • 1
    The subtitle mentions -f ("all but the last component must exist") and the examples use -e ("all components must exist"), which is a bit confusing. – isanae Jul 19 '16 at 4:46
  • 2
    @AaronCicali This is definitely not the answer you need, because it's dangerously wrong. You must not resolve symbolic links. The target of the symbolic link may change between the time you check it and the time you use it. (It's a well-known category of design errors). Besides, it doesn't make sense to resolve symlinks since rm acts on the argument itself, not its target. – Gilles Jul 19 '16 at 16:13
  • 1
    Hint: Use grep -q to keep grep from outputting matching lines. You still get the exit status so && and || still work exactly like you are used to. – a CVn Jul 21 '16 at 20:08
2

If you want to forbid paths completely, the simplest way is to test if the variable contains a slash (/). In bash:

if [[ "$1" = */* ]] ; then...

This will block all paths, though, including foo/bar. You could test for .. instead, but that would leave the possibility of symlinks pointing to directories outside the target path.

If you only want to allow deleting a single file, I don't think you should be using rm -r.


Also, depending on what you are doing, you could use the system's file permissions to only allow deleting files the user could delete themselves. Something like this:

su charlesingalls -c "rm /home/charlesingalls/'$1'"

Though as @Gilles commented, this has a quoting issue: it will fail if $1 contains a single quote, so the variable should be tested for that first (with e.g. if [[ "$1" = *\'* ]] ; then fail... or rather by whitelisting a sensible set of characters), or the file name passed through an environment variable with e.g.

file="$1" su charlesingalls -c 'rm "/home/charlesingalls/$file"'
  • Good point regarding the -r flag, that really shouldn't be there. Removing now... – Aaron Cicali Jul 19 '16 at 15:34
  • Note that your su command is broken because the quoting is wrong. You're executing a command with the argument interpolated as a shell snippet. E.g. if the argument is $(touch foo) then your code runs touch foo. – Gilles Jul 19 '16 at 16:16
  • @Gilles, crap, I knew there had to be something wrong. nested quoting is not my favourite friend. I think the current version works better, double quotes will evaluate the variable in the outer shell, and single quotes keep it from being evaluated in the inner shell. No warranty. – ilkkachu Jul 19 '16 at 16:29
  • @ilkkachu That version fails if the argument contains a single quote. (But only in that case, which makes it a lot easier to validate than when using double quotes.) It's impossible to directly interpolate an arbitrary string. You need to either massage the string (e.g. replace all ' by '\'') or pass it through another channel such as an environment variable (which is what I'd do here: file_to_remove="$1" su -c 'rm "/home/charlesingalls/$file_to_remove"'). It turns out that the file name is supposed to be a virtual host name, so rejecting all special characters would also be ok here. – Gilles Jul 19 '16 at 18:18

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.