From 45060ec40f0e9790d5e6a503486867951df71300 Mon Sep 17 00:00:00 2001 From: netblue30 Date: Tue, 20 Oct 2015 11:01:40 -0400 Subject: testing and fixes --- src/firejail/cgroup.c | 2 + src/firejail/firejail.h | 1 + src/firejail/fs_bin.c | 6 +- src/firejail/fs_etc.c | 2 + src/firejail/fs_home.c | 6 ++ src/firejail/join.c | 23 ++++- src/firejail/main.c | 6 ++ src/firejail/netfilter.c | 2 + src/firejail/output.c | 1 + src/firejail/profile.c | 23 +---- src/firejail/util.c | 20 ++++ test/extract_command.exp | 16 ++-- test/invalid_filename.exp | 236 ++++++++++++++++++++++++++++++++++++++++++++++ test/private.exp | 2 +- test/profile_apps.exp | 48 ---------- test/test.sh | 6 +- 16 files changed, 317 insertions(+), 83 deletions(-) create mode 100755 test/invalid_filename.exp delete mode 100755 test/profile_apps.exp 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` -- cgit v1.2.3-54-g00ecf