Merge pull request #7151 from kmk3/improve-xdg-macros

feature: allow subpaths in xdg macros

src/firejail/firejail.h

@@ -542,10 +542,10 @@ uint32_t arp_assign(const char *dev, Bridge *br);
 
 // macros.c
 char *expand_macros(const char *path);
-char *resolve_macro(const char *name);
+char *resolve_macro(const char *path);
 void invalid_filename(const char *fname, int globbing);
-int is_macro(const char *name);
-int macro_id(const char *name);
+int is_macro(const char *path);
+int macro_id(const char *path);
 
 
 // util.c

src/firejail/fs_whitelist.c

@@ -596,8 +596,8 @@ void fs_whitelist(void) {
 		if (is_macro(expanded) && macro_id(expanded) > -1) {
 			if (!nowhitelist_flag && (have_topdir(cfg.homedir, topdirs) || add_topdir(cfg.homedir, topdirs, expanded)) && !arg_quiet) {
 				fprintf(stderr, "***\n");
-				fprintf(stderr, "*** Warning: cannot whitelist %s directory\n", expanded);
-				fprintf(stderr, "*** Any file saved in this directory will be lost when the sandbox is closed.\n");
+				fprintf(stderr, "*** Warning: cannot whitelist %s path\n", expanded);
+				fprintf(stderr, "*** Any file saved in this path will be lost when the sandbox is closed.\n");
 				fprintf(stderr, "***\n");
 			}
 			entry = entry->next;

src/firejail/macros.c

@@ -31,6 +31,18 @@ typedef struct macro_t {
 } Macro;
 
 Macro macro[] = {
+	{
+		"${DESKTOP}",
+		"XDG_DESKTOP_DIR=\"$HOME/",
+		{"Desktop", "Рабочий стол", "Bureau", "Scrivania", "Escritorio", "Área de trabalho", "Schreibtisch"}
+	},
+
+	{
+		"${DOCUMENTS}",
+		"XDG_DOCUMENTS_DIR=\"$HOME/",
+		{"Documents", "Документы", "Documenti", "Documentos", "Dokumente"}
+	},
+
 	{
 		"${DOWNLOADS}",
 		"XDG_DOWNLOAD_DIR=\"$HOME/",
@@ -43,12 +55,6 @@ Macro macro[] = {
 		{"Music", "Музыка", "Musique", "Musica", "Música", "Musik"}
 	},
 
-	{
-		"${VIDEOS}",
-		"XDG_VIDEOS_DIR=\"$HOME/",
-		{"Videos", "Видео", "Vidéos", "Video", "Vídeos"}
-	},
-
 	{
 		"${PICTURES}",
 		"XDG_PICTURES_DIR=\"$HOME/",
@@ -56,25 +62,20 @@ Macro macro[] = {
 	},
 
 	{
-		"${DESKTOP}",
-		"XDG_DESKTOP_DIR=\"$HOME/",
-		{"Desktop", "Рабочий стол", "Bureau", "Scrivania", "Escritorio", "Área de trabalho", "Schreibtisch"}
-	},
-
-	{
-		"${DOCUMENTS}",
-		"XDG_DOCUMENTS_DIR=\"$HOME/",
-		{"Documents", "Документы", "Documenti", "Documentos", "Dokumente"}
+		"${VIDEOS}",
+		"XDG_VIDEOS_DIR=\"$HOME/",
+		{"Videos", "Видео", "Vidéos", "Video", "Vídeos"}
 	},
 
 	{ 0 }
 };
 
 // return -1 if not found
-int macro_id(const char *name) {
+int macro_id(const char *path) {
 	int i = 0;
 	while (macro[i].name != NULL) {
-		if (strcmp(name, macro[i].name) == 0)
+		size_t len = strlen(macro[i].name);
+		if (strncmp(path, macro[i].name, len) == 0)
 			return i;
 		i++;
 	}
@@ -82,12 +83,12 @@ int macro_id(const char *name) {
 	return -1;
 }
 
-int is_macro(const char *name) {
-	assert(name);
-	int len = strlen(name);
+int is_macro(const char *path) {
+	assert(path);
+	int len = strlen(path);
 	if (len <= 4)
 		return 0;
-	if (*name == '$' && name[1] == '{' && name[len - 1] == '}')
+	if (*path == '$' && path[1] == '{' && strchr(&path[2], '}'))
 		return 1;
 	return 0;
 }
@@ -175,17 +176,25 @@ static char *resolve_hardcoded(char *entries[]) {
 }
 
 // returns mallocated memory
-char *resolve_macro(const char *name) {
+char *resolve_macro(const char *path) {
 	char *rv = NULL;
-	int id = macro_id(name);
+	int id = macro_id(path);
 	if (id == -1)
 		return NULL;
 
-	rv = resolve_xdg(macro[id].xdg);
-	if (rv == NULL)
-		rv = resolve_hardcoded(macro[id].translation);
+	char *directory = resolve_xdg(macro[id].xdg);
+	if (!directory)
+		directory = resolve_hardcoded(macro[id].translation);
+	if (!directory)
+		return NULL;
+
+	size_t len = strlen(macro[id].name);
+	if (asprintf(&rv, "%s/%s%s", cfg.homedir, directory, path + len) == -1)
+		errExit("asprintf");
+	free(directory);
+
 	if (rv && arg_debug)
-		printf("Directory %s resolved as %s\n", name, rv);
+		printf("Path %s resolved as %s\n", path, rv);
 
 	return rv;
 }
@@ -200,66 +209,52 @@ char *expand_macros(const char *path) {
 
 	int called_as_root = 0;
 
-	if(geteuid() == 0)
+	if (geteuid() == 0)
 		called_as_root = 1;
 
-	if(called_as_root) {
+	if (called_as_root)
 		EUID_USER();
-	}
 
 	EUID_ASSERT();
 
 	// Replace home macro
-	char *new_name = NULL;
+	char *rv = NULL;
 	if (strncmp(path, "$HOME", 5) == 0) {
 		fprintf(stderr, "Error: $HOME is not allowed in profile files, please replace it with ${HOME}\n");
 		exit(1);
 	}
 	else if (strncmp(path, "${HOME}", 7) == 0) {
-		if (asprintf(&new_name, "%s%s", cfg.homedir, path + 7) == -1)
+		if (asprintf(&rv, "%s%s", cfg.homedir, path + 7) == -1)
 			errExit("asprintf");
-		if(called_as_root)
-			EUID_ROOT();
-		return new_name;
+		goto out;
 	}
 	else if (*path == '~') {
-		if (asprintf(&new_name, "%s%s", cfg.homedir, path + 1) == -1)
+		if (asprintf(&rv, "%s%s", cfg.homedir, path + 1) == -1)
 			errExit("asprintf");
-		if(called_as_root)
-			EUID_ROOT();
-		return new_name;
+		goto out;
 	}
 	else if (strncmp(path, "${CFG}", 6) == 0) {
-		if (asprintf(&new_name, "%s%s", SYSCONFDIR, path + 6) == -1)
+		if (asprintf(&rv, "%s%s", SYSCONFDIR, path + 6) == -1)
 			errExit("asprintf");
-		if(called_as_root)
-			EUID_ROOT();
-		return new_name;
+		goto out;
 	}
 	else if (strncmp(path, "${RUNUSER}", 10) == 0) {
-		if (asprintf(&new_name, "/run/user/%u%s", getuid(), path + 10) == -1)
+		if (asprintf(&rv, "/run/user/%u%s", getuid(), path + 10) == -1)
 			errExit("asprintf");
-		if(called_as_root)
-			EUID_ROOT();
-		return new_name;
+		goto out;
 	}
 	else {
-		char *directory = resolve_macro(path);
-		if (directory) {
-			if (asprintf(&new_name, "%s/%s", cfg.homedir, directory) == -1)
-				errExit("asprintf");
-			if(called_as_root)
-				EUID_ROOT();
-			free(directory);
-			return new_name;
-		}
+		rv = resolve_macro(path);
+		if (rv)
+			goto out;
 	}
 
-	char *rv = strdup(path);
+	assert(rv == NULL);
+	rv = strdup(path);
 	if (!rv)
 		errExit("strdup");
-
-	if(called_as_root)
+out:
+	if (called_as_root)
 		EUID_ROOT();
 
 	return rv;
@@ -279,7 +274,7 @@ void invalid_filename(const char *fname, int globbing) {
 	else {
 		int id = macro_id(fname);
 		if (id != -1)
-			return;
+			ptr = fname + strlen(macro[id].name);
 	}
 
 	reject_meta_chars(ptr, globbing);

test/fs/fs.sh

@@ -105,6 +105,27 @@ rm -f ~/Music/_firejail_test_file
 rm -f ~/Pictures/_firejail_test_file
 rm -f ~/Videos/_firejail_test_file
 
+mkdir -p ~/Desktop/_firejail_test_dir/a
+mkdir -p ~/Desktop/_firejail_test_dir/b
+mkdir -p ~/Documents/_firejail_test_dir/a
+mkdir -p ~/Documents/_firejail_test_dir/b
+mkdir -p ~/Downloads/_firejail_test_dir/a
+mkdir -p ~/Downloads/_firejail_test_dir/b
+mkdir -p ~/Music/_firejail_test_dir/a
+mkdir -p ~/Music/_firejail_test_dir/b
+mkdir -p ~/Pictures/_firejail_test_dir/a
+mkdir -p ~/Pictures/_firejail_test_dir/b
+mkdir -p ~/Videos/_firejail_test_dir/a
+mkdir -p ~/Videos/_firejail_test_dir/b
+echo "TESTING: macro subpaths (test/fs/macro-subpath.exp)"
+./macro-subpath.exp
+rm -fr ~/Desktop/_firejail_test_dir
+rm -fr ~/Documents/_firejail_test_dir
+rm -fr ~/Downloads/_firejail_test_dir
+rm -fr ~/Music/_firejail_test_dir
+rm -fr ~/Pictures/_firejail_test_dir
+rm -fr ~/Videos/_firejail_test_dir
+
 echo "TESTING: whitelist empty (test/fs/whitelist-empty.exp)"
 ./whitelist-empty.exp
 

test/fs/macro-subpath-blacklist.profile

@@ -0,0 +1,6 @@
+blacklist ${DESKTOP}/_firejail_test_dir
+blacklist ${DOCUMENTS}/_firejail_test_dir
+blacklist ${DOWNLOADS}/_firejail_test_dir
+blacklist ${MUSIC}/_firejail_test_dir
+blacklist ${PICTURES}/_firejail_test_dir
+blacklist ${VIDEOS}/_firejail_test_dir

test/fs/macro-subpath-readonly.profile

@@ -0,0 +1,6 @@
+read-only ${DESKTOP}/_firejail_test_dir
+read-only ${DOCUMENTS}/_firejail_test_dir
+read-only ${DOWNLOADS}/_firejail_test_dir
+read-only ${MUSIC}/_firejail_test_dir
+read-only ${PICTURES}/_firejail_test_dir
+read-only ${VIDEOS}/_firejail_test_dir

test/fs/macro-subpath-whitelist.profile

@@ -0,0 +1,6 @@
+whitelist ${DESKTOP}/_firejail_test_dir/a
+whitelist ${DOCUMENTS}/_firejail_test_dir/a
+whitelist ${DOWNLOADS}/_firejail_test_dir/a
+whitelist ${MUSIC}/_firejail_test_dir/a
+whitelist ${PICTURES}/_firejail_test_dir/a
+whitelist ${VIDEOS}/_firejail_test_dir/a

test/fs/macro-subpath.exp

@@ -0,0 +1,200 @@
+#!/usr/bin/expect -f
+# This file is part of Firejail project
+# Copyright (C) 2014-2026 Firejail Authors
+# License GPL v2
+
+set timeout 3
+spawn $env(SHELL)
+match_max 100000
+
+# Test that macros work with subpaths (see #2359).
+send -- "firejail --profile=./macro-subpath-whitelist.profile ls \
+	~/Desktop/_firejail_test_dir \
+	~/Documents/_firejail_test_dir \
+	~/Downloads/_firejail_test_dir \
+	~/Music/_firejail_test_dir \
+	~/Pictures/_firejail_test_dir \
+	~/Videos/_firejail_test_dir \
+\r"
+
+expect {
+	timeout {puts "TESTING ERROR 0\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 1\n";exit}
+	"Desktop/_firejail_test_dir/a" {}
+	"Desktop/_firejail_test_dir/b" {puts "TESTING ERROR 1.1\n";exit}
+}
+expect {
+	timeout {puts "TESTING ERROR 2\n";exit}
+	"Documents/_firejail_test_dir/a" {}
+	"Documents/_firejail_test_dir/b" {puts "TESTING ERROR 2.1\n";exit}
+}
+expect {
+	timeout {puts "TESTING ERROR 3\n";exit}
+	"Downloads/_firejail_test_dir/a" {}
+	"Downloads/_firejail_test_dir/b" {puts "TESTING ERROR 3.1\n";exit}
+}
+expect {
+	timeout {puts "TESTING ERROR 4\n";exit}
+	"Music/_firejail_test_dir/a" {}
+	"Music/_firejail_test_dir/b" {puts "TESTING ERROR 4.1\n";exit}
+}
+expect {
+	timeout {puts "TESTING ERROR 5\n";exit}
+	"Pictures/_firejail_test_dir/a" {}
+	"Pictures/_firejail_test_dir/b" {puts "TESTING ERROR 5.1\n";exit}
+}
+expect {
+	timeout {puts "TESTING ERROR 6\n";exit}
+	"Videos/_firejail_test_dir/a" {}
+	"Videos/_firejail_test_dir/b" {puts "TESTING ERROR 6.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Desktop/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 7\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 8\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 8.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Documents/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 9\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 10\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 10.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Downloads/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 11\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 12\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 12.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Music/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 13\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 14\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 14.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Pictures/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 15\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 16\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 16.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-blacklist.profile ls ~/Videos/_firejail_test_dir; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 17\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 18\n";exit}
+	"Permission denied" {}
+	-re {ret 0} {puts "TESTING ERROR 18.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Desktop/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 19\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 20\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 20.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Documents/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 21\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 22\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 22.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Downloads/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 23\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 24\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 24.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Music/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 25\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 26\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 26.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Pictures/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 27\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 28\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 28.1\n";exit}
+}
+after 100
+
+send -- "firejail --profile=./macro-subpath-readonly.profile touch ~/Videos/_firejail_test_dir/_firejail_test_file; echo ret \$?\r"
+expect {
+	timeout {puts "TESTING ERROR 29\n";exit}
+	-re "Child process initialized in \[0-9\]+.\[0-9\]+ ms"
+}
+expect {
+	timeout {puts "TESTING ERROR 30\n";exit}
+	"Read-only file system" {}
+	-re {ret 0} {puts "TESTING ERROR 30.1\n";exit}
+}
+after 100
+
+puts "\nall done\n"