Merge pull request #5856 from kmk3/standardize-name-checks

modif: Standardize and add missing name/hostname checks

src/firejail/main.c

@@ -2187,34 +2187,21 @@ 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) || has_cntrl_chars(cfg.name)) {
+ if (invalid_name(cfg.name)) {
  fprintf(stderr, "Error: invalid sandbox name\n");
  return 1;
  }
  }
  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,11 @@ 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;
+ }
+ if (invalid_name(cfg.name)) {
+ fprintf(stderr, "Error: invalid sandbox name\n");
  return 1;
  }
  }

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;
@@ -1165,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;
  }
 
@@ -1647,6 +1646,10 @@ 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: cannot be empty\n");
+ exit(1);
+ }
+ if (invalid_name(cfg.name)) {
  fprintf(stderr, "Error: invalid sandbox name\n");
  exit(1);
  }

src/firejail/util.c

@@ -1476,23 +1476,46 @@ int ascii_isxdigit(unsigned char c) {
  return ret;
 }
 
-// allow strict ASCII letters and numbers; names with only numbers are rejected; spaces are rejected
+// 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) {
  const char *c = name;
-
  int only_numbers = 1;
+
+ 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;
  }
- if (only_numbers)
+
+ // must end with alnum
+ --c;
+ if (!ascii_isalnum(*c))
  return 1;
 
- // restrict name to 64 chars max
- if (strlen(name) > 64)
+ if (only_numbers)
  return 1;
 
  return 0;

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