16 files changed, 317 insertions, 83 deletions

diff --git a/src/firejail/cgroup.c b/src/firejail/cgroup.c
index 4d64d3fd8..9e6a2e549 100644
--- a/src/firejail/cgroup.c
+++ b/src/firejail/cgroup.c
@@ -78,6 +78,8 @@ errout:
 
 
 void set_cgroup(const char *path) {
+        invalid_filename(path);
+        
         // path starts with /sys/fs/cgroup
         if (strncmp(path, "/sys/fs/cgroup", 14) != 0)
                 goto errout;
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 2e82dabc9..d3cfb1e96 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -296,6 +296,7 @@ void notify_other(int fd);
 char *expand_home(const char *path, const char* homedir);
 const char *gnu_basename(const char *path);
 uid_t pid_get_uid(pid_t pid);
+void invalid_filename(const char *fname);
 
 // fs_var.c
 void fs_var_log(void);  // mounting /var/log
diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c
index dcfdadb6b..668223755 100644
--- a/src/firejail/fs_bin.c
+++ b/src/firejail/fs_bin.c
@@ -35,6 +35,8 @@ static char *paths[] = {
 // return 1 if found, 0 if not found
 static char *check_dir_or_file(const char *name) {
         assert(name);
+        invalid_filename(name);
+        
         struct stat s;
         char *fname = NULL;
         
@@ -52,8 +54,10 @@ static char *check_dir_or_file(const char *name) {
                 i++;
         }
 
-        if (!fname)
+        if (!fname) {
+                fprintf(stderr, "Warning: file %s not found\n", name);
                 return NULL;
+        }
         
         free(fname);
         return paths[i];
diff --git a/src/firejail/fs_etc.c b/src/firejail/fs_etc.c
index ea6f4fe6d..8e5fe1b86 100644
--- a/src/firejail/fs_etc.c
+++ b/src/firejail/fs_etc.c
@@ -26,6 +26,8 @@
 
 static void check_dir_or_file(const char *name) {
         assert(name);
+        invalid_filename(name);
+        
         struct stat s;
         char *fname;
         if (asprintf(&fname, "/etc/%s", name) == -1)
diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c
index 21a2e83e5..1144e90e8 100644
--- a/src/firejail/fs_home.c
+++ b/src/firejail/fs_home.c
@@ -254,6 +254,10 @@ void fs_private(void) {
 static void check_dir_or_file(const char *name) {
         assert(name);
         struct stat s;
+        
+        invalid_filename(name);
+        
+        
         char *fname = expand_home(name, cfg.homedir);
         if (!fname) {
                 fprintf(stderr, "Error: file %s not found.\n", name);
@@ -318,6 +322,8 @@ void fs_check_home_list(void) {
 
 // check new private home directory (--private= option) - exit if it fails
 void fs_check_private_dir(void) {
+        invalid_filename(cfg.home_private);
+        
         // Expand the home directory
         char *tmp = expand_home(cfg.home_private, cfg.homedir);
         cfg.home_private = realpath(tmp, NULL);
diff --git a/src/firejail/join.c b/src/firejail/join.c
index acd17366a..35e302bf0 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -306,10 +306,25 @@ void join(pid_t pid, const char *homedir, int argc, char **argv, int index) {
                 if (setenv("PROMPT_COMMAND", "export PS1=\"\\[\\e[1;32m\\][\\u@\\h \\W]\\$\\[\\e[0m\\] \"", 1) < 0)
                         errExit("setenv");
 
-                // run icmdline trough /bin/bash
-                if (cfg.command_line == NULL)
-                        // replace the process with a regular bash session
-                        execlp("/bin/bash", "/bin/bash", NULL);
+                // run cmdline trough /bin/bash
+                if (cfg.command_line == NULL) {
+                        struct stat s;
+
+                        // replace the process with a shell
+                        if (stat("/bin/bash", &s) == 0)
+                                execlp("/bin/bash", "/bin/bash", NULL);
+                        else if (stat("/usr/bin/zsh", &s) == 0)
+                                execlp("/usr/bin/zsh", "/usr/bin/zsh", NULL);
+                        else if (stat("/bin/csh", &s) == 0)
+                                execlp("/bin/csh", "/bin/csh", NULL);
+                        else if (stat("/bin/sh", &s) == 0)
+                                execlp("/bin/sh", "/bin/sh", NULL);
+                        
+                        // no shell found, print an error and exit
+                        fprintf(stderr, "Error: no POSIX shell found\n");
+                        sleep(5);
+                        exit(1);
+                }
                 else {
                         // run the command supplied by the user
                         int cwd = 0;
diff --git a/src/firejail/main.c b/src/firejail/main.c
index 8d2664c16..422a39128 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -586,6 +586,7 @@ int main(int argc, char **argv) {
                                 fprintf(stderr, "Error: only a cgroup can be defined\n");
                                 exit(1);
                         }
+                        
                         arg_cgroup = 1;
                         cfg.cgroup = strdup(argv[i] + 9);
                         if (!cfg.cgroup)
@@ -683,6 +684,8 @@ int main(int argc, char **argv) {
                                 fprintf(stderr, "Error: --noprofile and --profile options are mutually exclusive\n");
                                 exit(1);
                         }
+                        invalid_filename(argv[i] + 10);
+                        
                         // multiple profile files are allowed!
                         char *ptr = argv[i] + 10;
                         if (is_dir(ptr) || is_link(ptr) || strstr(ptr, "..")) {
@@ -712,6 +715,7 @@ int main(int argc, char **argv) {
                                 fprintf(stderr, "Error: --overlay and --chroot options are mutually exclusive\n");
                                 exit(1);
                         }
+                        invalid_filename(argv[i] + 9);
                         
                         // extract chroot dirname
                         cfg.chrootdir = argv[i] + 9;
@@ -1042,6 +1046,8 @@ int main(int argc, char **argv) {
                                 fprintf(stderr, "Error: --shell=none was already specified.\n");
                                 return 1;
                         }
+                        invalid_filename(argv[i] + 8);
+                        
                         if (arg_csh || arg_zsh || cfg.shell) {
                                 fprintf(stderr, "Error: only one user shell can be specified\n");
                                 return 1;
diff --git a/src/firejail/netfilter.c b/src/firejail/netfilter.c
index 8601a5696..3f667c871 100644
--- a/src/firejail/netfilter.c
+++ b/src/firejail/netfilter.c
@@ -39,6 +39,8 @@ static char *client_filter =
 "COMMIT\n";
 
 void check_netfilter_file(const char *fname) {
+        invalid_filename(fname);
+        
         if (is_dir(fname) || is_link(fname) || strstr(fname, "..")) {
                 fprintf(stderr, "Error: invalid network filter file\n");
                 exit(1);
diff --git a/src/firejail/output.c b/src/firejail/output.c
index 94a05ac4a..c2ab9f157 100644
--- a/src/firejail/output.c
+++ b/src/firejail/output.c
@@ -31,6 +31,7 @@ void check_output(int argc, char **argv) {
         for (i = 1; i < argc; i++) {
                 if (strncmp(argv[i], "--output=", 9) == 0) {
                         found = 1;
+                        invalid_filename(argv[i] + 9);
                         outfile = argv[i] + 9;
 
                         // do not accept directories, links, and files with ".."
diff --git a/src/firejail/profile.c b/src/firejail/profile.c
index 2e3790b3c..e6c31bc0a 100644
--- a/src/firejail/profile.c
+++ b/src/firejail/profile.c
@@ -61,23 +61,6 @@ int profile_find(const char *name, const char *dir) {
 //***************************************************
 // run-time profiles
 //***************************************************
-static void check_file_name(char *ptr, int lineno) {
-        if (strncmp(ptr, "${HOME}", 7) == 0)
-                ptr += 7;
-        else if (strncmp(ptr, "${PATH}", 7) == 0)
-                ptr += 7;
-
-        int len = strlen(ptr);
-        // file globbing ('*') is allowed
-        if (strcspn(ptr, "\\&!?\"'<>%^(){}[];,") != (size_t)len) {
-                if (lineno == 0)
-                        fprintf(stderr, "Error: \"%s\" is an invalid filename\n", ptr);
-                else
-                        fprintf(stderr, "Error: line %d in the custom profile is invalid\n", lineno);
-                exit(1);
-        }
-}
-
 
 // check profile line; if line == 0, this was generated from a command line option
 // return 1 if the command is to be added to the linked list of profile commands
@@ -281,8 +264,8 @@ int profile_check_line(char *ptr, int lineno) {
                 }
                 
                 // check directories
-                check_file_name(dname1, lineno);
-                check_file_name(dname2, lineno);
+                invalid_filename(dname1);
+                invalid_filename(dname2);
                 if (strstr(dname1, "..") || strstr(dname2, "..")) {
                         fprintf(stderr, "Error: invalid file name.\n");
                         exit(1);
@@ -361,7 +344,7 @@ int profile_check_line(char *ptr, int lineno) {
         }
 
         // some characters just don't belong in filenames
-        check_file_name(ptr, lineno);
+        invalid_filename(ptr);
         if (strstr(ptr, "..")) {
                 if (lineno == 0)
                         fprintf(stderr, "Error: \"%s\" is an invalid filename\n", ptr);
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 9ad937f55..d2e6c2799 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -535,3 +535,23 @@ uid_t pid_get_uid(pid_t pid) {
         }
         return rv;
 }
+
+void invalid_filename(const char *fname) {
+        assert(fname);
+        const char *ptr = fname;
+        
+        if (arg_debug)
+                printf("Checking filename %s\n", fname);
+        
+        if (strncmp(ptr, "${HOME}", 7) == 0)
+                ptr = fname + 7;
+        else if (strncmp(ptr, "${PATH}", 7) == 0)
+                ptr = fname + 7;
+
+        int len = strlen(ptr);
+        // file globbing ('*') is allowed
+        if (strcspn(ptr, "\\&!?\"'<>%^(){}[];,") != (size_t)len) {
+                fprintf(stderr, "Error: \"%s\" is an invalid filename\n", ptr);
+                exit(1);
+        }
+}
diff --git a/test/extract_command.exp b/test/extract_command.exp
index c49614b84..b4a1eeeb5 100755
--- a/test/extract_command.exp
+++ b/test/extract_command.exp
@@ -4,20 +4,24 @@ set timeout 10
 spawn $env(SHELL)
 match_max 100000
 
-send -- "firejail --debug /usr/bin/firefox www.gentoo.org\r"
+send -- "firejail --debug ls -al\r"
 expect {
         timeout {puts "TESTING ERROR 0\n";exit}
-        "Reading profile /etc/firejail/firefox.profile"
+        "Reading profile /etc/firejail/generic.profile"
 }
 expect {
         timeout {puts "TESTING ERROR 1\n";exit}
-        "Starting /usr/bin/firefox"
+        "Starting ls -al"
 }
 expect {
-        timeout {puts "TESTING ERROR 1\n";exit}
+        timeout {puts "TESTING ERROR 2\n";exit}
         "Child process initialized"
 }
-sleep 5
+expect {
+        timeout {puts "TESTING ERROR 2\n";exit}
+        "parent is shutting down, bye"
+}
+sleep 1
 
-puts "\n"
+puts "\nall done\n"
 
diff --git a/test/invalid_filename.exp b/test/invalid_filename.exp
new file mode 100755
index 000000000..ca35262f8
--- /dev/null
+++ b/test/invalid_filename.exp
@@ -0,0 +1,236 @@
+#!/usr/bin/expect -f
+
+#invalid_filename checks:
+#
+#--bind (two files) - profile.c - Note: The test is not implemented here, need to be root to test it
+#--blacklist - profile.c
+#--cgroup - cgroup.c
+#--chroot - main.c
+#--netfilter - netfilter.c
+#--output - output.c
+#--private - fs_home.c
+#--privte-bin (list) - fs_bin.c
+#--private-keep/--private-home (list) - fs_home.c
+#--private-etc (list) - fs_etc.c
+#--profile - main.c
+#--read_only - profile.c
+#--shell - main.c
+#--tmpfs - profile.c
+#--white-list
+
+
+set timeout 10
+spawn $env(SHELL)
+match_max 100000
+
+send -- "firejail --debug --noprofile --blacklist=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 1.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 1.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 1.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --cgroup=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 2.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 2.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 2.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --chroot=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 3.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 3.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 3.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --netfilter=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 4.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 4.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 4.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --output=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 5.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 5.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --private=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 6.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 6.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 6.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --private-bin=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 7.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 7.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 7.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --noprofile --private-home=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 8.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 8.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 8.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+
+send -- "firejail --debug --noprofile --private-etc=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 9.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 9.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 9.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --profile=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 10.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 10.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 10.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --read-only=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 11.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 11.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 11.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --shell=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 12.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 12.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 12.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --tmpfs=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 13.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 13.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 13.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+send -- "firejail --debug --whitelist=\"bla&&bla\"\r"
+expect {
+        timeout {puts "TESTING ERROR 14.1\n";exit}
+        "Checking filename bla&&bla"
+}
+expect {
+        timeout {puts "TESTING ERROR 14.2\n";exit}
+        "Error:"
+}
+expect {
+        timeout {puts "TESTING ERROR 14.3\n";exit}
+        "is an invalid filename"
+}
+after 100
+
+
+
+puts "\nall done\n"
+
diff --git a/test/private.exp b/test/private.exp
index 6f9dadf8f..8a6796802 100755
--- a/test/private.exp
+++ b/test/private.exp
@@ -12,7 +12,7 @@ if { $argc != 1 } {
 }
 
 # testing profile and private
-send -- "firejail --private --profile=/etc/firejail/firefox.profile\r"
+send -- "firejail --private --profile=/etc/firejail/generic.profile\r"
 expect {
         timeout {puts "TESTING ERROR 0\n";exit}
         "Child process initialized"
diff --git a/test/profile_apps.exp b/test/profile_apps.exp
deleted file mode 100755
index c57b31489..000000000
--- a/test/profile_apps.exp
+++ /dev/null
@@ -1,48 +0,0 @@
-#!/usr/bin/expect -f
-
-set timeout 10
-spawn $env(SHELL)
-match_max 100000
-
-# firefox
-send -- "firejail --profile=/etc/firejail/firefox.profile\r"
-expect {
-        timeout {puts "TESTING ERROR 0\n";exit}
-        "Child process initialized"
-}
-sleep 1
-send -- "exit\r"
-sleep 1
-
-# iceweasel
-send -- "firejail --profile=/etc/firejail/iceweasel.profile\r"
-expect {
-        timeout {puts "TESTING ERROR 1\n";exit}
-        "Child process initialized"
-}
-sleep 1
-send -- "exit\r"
-sleep 1
-
-# evince
-send -- "firejail --profile=/etc/firejail/evince.profile\r"
-expect {
-        timeout {puts "TESTING ERROR 2\n";exit}
-        "Child process initialized"
-}
-sleep 1
-send -- "exit\r"
-sleep 1
-
-# midori
-send -- "firejail --profile=/etc/firejail/midori.profile\r"
-expect {
-        timeout {puts "TESTING ERROR 3\n";exit}
-        "Child process initialized"
-}
-sleep 1
-send -- "exit\r"
-sleep 1
-
-
-puts "\n"
diff --git a/test/test.sh b/test/test.sh
index 7d43a5786..c986b5f29 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -12,6 +12,9 @@ done
 
 ./fscheck.sh
 
+echo "TESTING: invalid filename"
+./invalid_filename.exp
+
 echo "TESTING: environment variables"
 ./env.exp
 
@@ -276,9 +279,6 @@ echo "TESTING: profile read-only"
 echo "TESTING: profile tmpfs"
 ./profile_tmpfs.exp
 
-echo "TESTING: profile applications"
-./profile_apps.exp
-
 echo "TESTING: private"
 ./private.exp `whoami`