From 10e9596109514b6160b79f426c67e41a01bb16c4 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 18:23:38 -0300 Subject: main.c: remove redundant has_cntrl_chars check The `invalid_name` function does not allow control characters. Added on commit d349a2ff8 ("Forbid control chars in names", 2023-03-03) / PR #5708. --- src/firejail/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/firejail/main.c b/src/firejail/main.c index 1835d8de2..715123279 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2190,7 +2190,7 @@ int main(int argc, char **argv, char **envp) { fprintf(stderr, "Error: please provide a name for sandbox\n"); return 1; } - if (invalid_name(cfg.name) || has_cntrl_chars(cfg.name)) { + if (invalid_name(cfg.name)) { fprintf(stderr, "Error: invalid sandbox name\n"); return 1; } -- cgit v1.2.3-54-g00ecf From 052de475b9c6fc0c4670fc202ed24f75d12b1148 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 17:52:48 -0300 Subject: util.c: increase name max length from 64 to 253 To match the hostname check in src/firejail/main.c. --- src/firejail/util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/firejail/util.c b/src/firejail/util.c index a0af3d4bf..78704fa64 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -1479,8 +1479,11 @@ int ascii_isxdigit(unsigned char c) { // allow strict ASCII letters and numbers; names with only numbers are rejected; spaces are rejected int invalid_name(const char *name) { const char *c = name; - int only_numbers = 1; + + if (strlen(name) > 253) + return 1; + while (*c) { if (!ascii_isalnum(*c)) return 1; @@ -1491,10 +1494,6 @@ int invalid_name(const char *name) { if (only_numbers) return 1; - // restrict name to 64 chars max - if (strlen(name) > 64) - return 1; - return 0; } -- cgit v1.2.3-54-g00ecf From 94738819b17cd03adbce6284a87f5fd0bb28d077 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 18:21:22 -0300 Subject: util.c: check first/last char and allow extra chars In `invalid_name`. --- src/firejail/util.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/firejail/util.c b/src/firejail/util.c index 78704fa64..6c79c050e 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -1476,7 +1476,8 @@ int ascii_isxdigit(unsigned char c) { return ret; } -// allow strict ASCII letters and numbers; names with only numbers are rejected; spaces are rejected +// Allow only ASCII letters, digits and a few special characters; names with +// only numbers are rejected; spaces and control characters are rejected. int invalid_name(const char *name) { const char *c = name; int only_numbers = 1; @@ -1484,13 +1485,34 @@ int invalid_name(const char *name) { if (strlen(name) > 253) return 1; + // must start with alnum + if (!ascii_isalnum(*c)) + return 1; + if (!ascii_isdigit(*c)) + only_numbers = 0; + ++c; + while (*c) { - if (!ascii_isalnum(*c)) - return 1; - if (!ascii_isdigit(*c)) + switch (*c) { + case '-': + case '.': + case '_': only_numbers = 0; + break; + default: + if (!ascii_isalnum(*c)) + return 1; + if (!ascii_isdigit(*c)) + only_numbers = 0; + } ++c; } + + // must end with alnum + --c; + if (!ascii_isalnum(*c)) + return 1; + if (only_numbers) return 1; -- cgit v1.2.3-54-g00ecf From eb6c61154c37605842c3fed906405e3d9653f1ae Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 18:30:25 -0300 Subject: Standardize name/hostname checks Changes: * Use only `invalid_name` to check the name and hostname instead of ad-hoc checks * Standardize empty/invalid error messages for name/hostname Note: This makes the hostname validation less strict, though it still forbids control characters and only numbers. Relates to #5578 #5708. See also commit b4ffaa207 ("merges; more on cleaning up esc chars", 2023-02-14). --- src/firejail/main.c | 23 +++++------------------ src/firejail/profile.c | 17 ++++------------- 2 files changed, 9 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/firejail/main.c b/src/firejail/main.c index 715123279..df1c81f3a 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2187,7 +2187,7 @@ int main(int argc, char **argv, char **envp) { else if (strncmp(argv[i], "--name=", 7) == 0) { cfg.name = argv[i] + 7; if (strlen(cfg.name) == 0) { - fprintf(stderr, "Error: please provide a name for sandbox\n"); + fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); return 1; } if (invalid_name(cfg.name)) { @@ -2197,24 +2197,11 @@ int main(int argc, char **argv, char **envp) { } else if (strncmp(argv[i], "--hostname=", 11) == 0) { cfg.hostname = argv[i] + 11; - size_t len = strlen(cfg.hostname); - if (len == 0 || len > 253) { - fprintf(stderr, "Error: please provide a valid hostname for sandbox, with maximum length of 253 ASCII characters\n"); + if (strlen(cfg.hostname) == 0) { + fprintf(stderr, "Error: invalid hostname: cannot be empty\n"); return 1; } - int invalid = invalid_name(cfg.hostname); - char* hostname = cfg.hostname; - while (*hostname && !invalid) { - invalid = invalid || !( - (*hostname >= 'a' && *hostname <= 'z') || - (*hostname >= 'A' && *hostname <= 'Z') || - (*hostname >= '0' && *hostname <= '9') || - (*hostname == '-' || *hostname == '.')); - hostname++; - } - invalid = invalid || cfg.hostname[0] == '-'; // must not start with - - invalid = invalid || cfg.hostname[len - 1] == '-'; // must not end with - - if (invalid) { + if (invalid_name(cfg.hostname)) { fprintf(stderr, "Error: invalid hostname\n"); return 1; } @@ -2847,7 +2834,7 @@ int main(int argc, char **argv, char **envp) { // set sandbox name and start normally cfg.name = argv[i] + 16; if (strlen(cfg.name) == 0) { - fprintf(stderr, "Error: please provide a name for sandbox\n"); + fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); return 1; } } diff --git a/src/firejail/profile.c b/src/firejail/profile.c index 202bcf4da..139ce0580 100644 --- a/src/firejail/profile.c +++ b/src/firejail/profile.c @@ -326,22 +326,13 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { } // sandbox name else if (strncmp(ptr, "name ", 5) == 0) { - int only_numbers = 1; cfg.name = ptr + 5; if (strlen(cfg.name) == 0) { - fprintf(stderr, "Error: invalid sandbox name\n"); + fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); exit(1); } - const char *c = cfg.name; - while (*c) { - if (!isdigit(*c)) { - only_numbers = 0; - break; - } - ++c; - } - if (only_numbers) { - fprintf(stderr, "Error: invalid sandbox name: it only contains digits\n"); + if (invalid_name(cfg.name)) { + fprintf(stderr, "Error: invalid sandbox name\n"); exit(1); } return 0; @@ -1647,7 +1638,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { // set sandbox name and start normally cfg.name = ptr + 14; if (strlen(cfg.name) == 0) { - fprintf(stderr, "Error: invalid sandbox name\n"); + fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); exit(1); } } -- cgit v1.2.3-54-g00ecf From a51968336dfa869be765e7109948725582971319 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 19:12:16 -0300 Subject: Add missing name/hostname checks Note that the sandbox name may also be set through the "join-or-start" option. Relates to #5578 #5708. --- src/firejail/main.c | 4 ++++ src/firejail/profile.c | 12 ++++++++++++ 2 files changed, 16 insertions(+) (limited to 'src') diff --git a/src/firejail/main.c b/src/firejail/main.c index df1c81f3a..070eb47f3 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2837,6 +2837,10 @@ int main(int argc, char **argv, char **envp) { fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); return 1; } + if (invalid_name(cfg.name)) { + fprintf(stderr, "Error: invalid sandbox name\n"); + return 1; + } } else if (strcmp(argv[i], "--deterministic-exit-code") == 0) { arg_deterministic_exit_code = 1; diff --git a/src/firejail/profile.c b/src/firejail/profile.c index 139ce0580..ae881664b 100644 --- a/src/firejail/profile.c +++ b/src/firejail/profile.c @@ -1156,6 +1156,14 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { // hostname if (strncmp(ptr, "hostname ", 9) == 0) { cfg.hostname = ptr + 9; + if (strlen(cfg.hostname) == 0) { + fprintf(stderr, "Error: invalid hostname: cannot be empty\n"); + exit(1); + } + if (invalid_name(cfg.hostname)) { + fprintf(stderr, "Error: invalid hostname\n"); + exit(1); + } return 0; } @@ -1641,6 +1649,10 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { fprintf(stderr, "Error: invalid sandbox name: cannot be empty\n"); exit(1); } + if (invalid_name(cfg.name)) { + fprintf(stderr, "Error: invalid sandbox name\n"); + exit(1); + } } else warning_feature_disabled("join"); -- cgit v1.2.3-54-g00ecf From 6489138a5685df83b7b8ce490b7fcc64cde3fb38 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Tue, 13 Jun 2023 21:16:17 -0300 Subject: docs: document NAME VALIDATION in firejail.txt --- src/firejail/util.c | 2 ++ src/man/firejail.txt | 24 ++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/firejail/util.c b/src/firejail/util.c index 6c79c050e..555486916 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -1476,6 +1476,8 @@ int ascii_isxdigit(unsigned char c) { return ret; } +// Note: Keep this in sync with NAME VALIDATION in src/man/firejail.txt. +// // Allow only ASCII letters, digits and a few special characters; names with // only numbers are rejected; spaces and control characters are rejected. int invalid_name(const char *name) { diff --git a/src/man/firejail.txt b/src/man/firejail.txt index 586ef9852..19fc94ebd 100644 --- a/src/man/firejail.txt +++ b/src/man/firejail.txt @@ -876,6 +876,8 @@ Print options end exit. \fB\-\-hostname=name Set sandbox hostname. .br +For valid names, see the \fBNAME VALIDATION\fR section. +.br .br Example: @@ -1180,7 +1182,9 @@ Switching to pid 1932, the first child process inside the sandbox .TP \fB\-\-join-or-start=name Join the sandbox identified by name or start a new one. -Same as "firejail --join=name" if sandbox with specified name exists, otherwise same as "firejail --name=name ..." +Same as "firejail --join=name" if sandbox with specified name exists, otherwise +same as "firejail --name=name ...". +See \fB\-\-name\fR for details. .br Note that in contrary to other join options there is respective profile option. @@ -1340,8 +1344,13 @@ $ firejail \-\-net=eth0 \-\-mtu=1492 \fB\-\-name=name Set sandbox name. Several options, such as \-\-join and \-\-shutdown, can use this name to identify a sandbox. -The name cannot contain only digits, as that is treated as a PID in the other options, such as in \-\-join. +The name cannot contain only digits, as that is treated as a PID in the other +options, such as in \-\-join. +.br +For valid names, see the \fBNAME VALIDATION\fR section. +.br +.br In case the name supplied by the user is already in use by another sandbox, Firejail will assign a new name as "name-PID", where PID is the process ID of the sandbox. This functionality can be disabled at run time in /etc/firejail/firejail.config file, by setting "name-change" flag to "no". @@ -3296,6 +3305,17 @@ Example: $ firejail --net=eth0 --x11=xephyr --xephyr-screen=640x480 firefox .br #endif +.\" Note: Keep this in sync with invalid_name() in src/firejail/util.c. +.SH NAME VALIDATION +For simplicity, the same name validation is used for multiple options. +Rules: +.PP +The name must be 1-253 characters long. +The name can only contain ASCII letters, digits and the special characters +"-._" (that is, the name cannot contain spaces or control characters). +The name cannot contain only digits. +The first and last characters must be an ASCII letter or digit and the name +may contain special characters in the middle. #ifdef HAVE_APPARMOR .SH APPARMOR .TP -- cgit v1.2.3-54-g00ecf