Fix local privilege escalation to root and sandbox bypasses in scheduler

(rdar://37836779, rdar://37836995, rdar://37837252, rdar://37837581)

diff --git a/man/cups-files.conf.man.in b/man/cups-files.conf.man.in
index 7b96d687d57f1aefb7262150d9ffe95b68171685..baf3cb6af7311a8771a8a7efe9d7658d211b4298 100644 (file)
--- a/man/cups-files.conf.man.in
+++ b/man/cups-files.conf.man.in
@@ -153,6 +153,11 @@ The server name may be included in filenames using the string "%s", for example:
 
 .fi
 The default is "/var/log/cups/page_log".
+.\"#PassEnv
+.TP 5
+\fBPassEnv \fIvariable \fR[ ... \fIvariable \fR]
+Passes the specified environment variable(s) to child processes.
+Note: the standard CUPS filter and backend environment variables cannot be overridden using this directive.
 .\"#RemoteRoot
 .TP 5
 \fBRemoteRoot \fIusername\fR
@@ -187,6 +192,11 @@ macOS uses its keychain database to store certificates and keys while other plat
 \fBServerRoot \fIdirectory\fR
 Specifies the directory containing the server configuration files.
 The default is "/etc/cups".
+.\"#SetEnv
+.TP 5
+\fBSetEnv \fIvariable value\fR
+Set the specified environment variable to be passed to child processes.
+Note: the standard CUPS filter and backend environment variables cannot be overridden using this directive.
 .\"#StateDir
 .TP 5
 \fBStateDir \fIdirectory\fR

diff --git a/man/cupsd.conf.man.in b/man/cupsd.conf.man.in
index 3ffc80e42b30a05aba1ec96bf83a07d015b98740..36c8493987dea9b2a8f5b140cac22148c89ab460 100644 (file)
--- a/man/cupsd.conf.man.in
+++ b/man/cupsd.conf.man.in
@@ -349,10 +349,6 @@ The default is "1048576" (1MB).
 \fBMultipleOperationTimeout \fIseconds\fR
 Specifies the maximum amount of time to allow between files in a multiple file print job.
 The default is "300" (5 minutes).
-.\"#PassEnv
-.TP 5
-\fBPassEnv \fIvariable \fR[ ... \fIvariable \fR]
-Passes the specified environment variable(s) to child processes.
 .\"#Policy
 .TP 5
 \fB<Policy \fIname\fB> \fR... \fB</Policy>\fR
@@ -433,10 +429,6 @@ Specifies what information is included in the Server header of HTTP responses.
 command.
 "Full" reports "CUPS 2.0.0 (UNAME) IPP/2.0".
 The default is "Minimal".
-.\"#SetEnv
-.TP 5
-\fBSetEnv \fIvariable value\fR
-Set the specified environment variable to be passed to child processes.
 .\"#SSLListen
 .TP 5
 \fBSSLListen \fIipv4-address\fB:\fIport\fR

diff --git a/scheduler/conf.c b/scheduler/conf.c
index 67a91e7a61b351dc069e02bd55157c213d3e4160..b51c6060c55c421761cac6cae690257a599be6bd 100644 (file)
--- a/scheduler/conf.c
+++ b/scheduler/conf.c
@@ -2929,13 +2929,10 @@ read_cupsd_conf(cups_file_t *fp)        /* I - File to read from */
                                        /* Line from file */
                        temp[HTTP_MAX_BUFFER],
                                        /* Temporary buffer for value */
-                       *value,         /* Pointer to value */
-                       *valueptr;      /* Pointer into value */
+                       *value;         /* Pointer to value */
   int                  valuelen;       /* Length of value */
   http_addrlist_t      *addrlist,      /* Address list */
                        *addr;          /* Current address */
-  cups_file_t          *incfile;       /* Include file */
-  char                 incname[1024];  /* Include filename */
 
 
  /*
@@ -2950,28 +2947,7 @@ read_cupsd_conf(cups_file_t *fp) /* I - File to read from */
     * Decode the directive...
     */
 
-    if (!_cups_strcasecmp(line, "Include") && value)
-    {
-     /*
-      * Include filename
-      */
-
-      if (value[0] == '/')
-        strlcpy(incname, value, sizeof(incname));
-      else
-        snprintf(incname, sizeof(incname), "%s/%s", ServerRoot, value);
-
-      if ((incfile = cupsFileOpen(incname, "rb")) == NULL)
-        cupsdLogMessage(CUPSD_LOG_ERROR,
-                       "Unable to include config file \"%s\" - %s",
-                       incname, strerror(errno));
-      else
-      {
-        read_cupsd_conf(incfile);
-       cupsFileClose(incfile);
-      }
-    }
-    else if (!_cups_strcasecmp(line, "<Location") && value)
+    if (!_cups_strcasecmp(line, "<Location") && value)
     {
      /*
       * <Location path>
@@ -3367,31 +3343,6 @@ read_cupsd_conf(cups_file_t *fp) /* I - File to read from */
        cupsdLogMessage(CUPSD_LOG_WARN, "Unknown ServerTokens %s on line %d of %s.",
                         value, linenum, ConfigurationFile);
     }
-    else if (!_cups_strcasecmp(line, "PassEnv") && value)
-    {
-     /*
-      * PassEnv variable [... variable]
-      */
-
-      for (; *value;)
-      {
-        for (valuelen = 0; value[valuelen]; valuelen ++)
-         if (_cups_isspace(value[valuelen]) || value[valuelen] == ',')
-           break;
-
-        if (value[valuelen])
-        {
-         value[valuelen] = '\0';
-         valuelen ++;
-       }
-
-        cupsdSetEnv(value, NULL);
-
-        for (value += valuelen; *value; value ++)
-         if (!_cups_isspace(*value) || *value != ',')
-           break;
-      }
-    }
     else if (!_cups_strcasecmp(line, "ServerAlias") && value)
     {
      /*
@@ -3420,30 +3371,6 @@ read_cupsd_conf(cups_file_t *fp) /* I - File to read from */
            break;
       }
     }
-    else if (!_cups_strcasecmp(line, "SetEnv") && value)
-    {
-     /*
-      * SetEnv variable value
-      */
-
-      for (valueptr = value; *valueptr && !isspace(*valueptr & 255); valueptr ++);
-
-      if (*valueptr)
-      {
-       /*
-        * Found a value...
-       */
-
-        while (isspace(*valueptr & 255))
-         *valueptr++ = '\0';
-
-        cupsdSetEnv(value, valueptr);
-      }
-      else
-        cupsdLogMessage(CUPSD_LOG_ERROR,
-                       "Missing value for SetEnv directive on line %d of %s.",
-                       linenum, ConfigurationFile);
-    }
     else if (!_cups_strcasecmp(line, "AccessLog") ||
              !_cups_strcasecmp(line, "CacheDir") ||
              !_cups_strcasecmp(line, "ConfigFilePerm") ||
@@ -3457,6 +3384,7 @@ read_cupsd_conf(cups_file_t *fp)  /* I - File to read from */
              !_cups_strcasecmp(line, "LogFilePerm") ||
              !_cups_strcasecmp(line, "LPDConfigFile") ||
              !_cups_strcasecmp(line, "PageLog") ||
+             !_cups_strcasecmp(line, "PassEnv") ||
              !_cups_strcasecmp(line, "Printcap") ||
              !_cups_strcasecmp(line, "PrintcapFormat") ||
              !_cups_strcasecmp(line, "RemoteRoot") ||
@@ -3466,6 +3394,7 @@ read_cupsd_conf(cups_file_t *fp)  /* I - File to read from */
              !_cups_strcasecmp(line, "ServerKey") ||
              !_cups_strcasecmp(line, "ServerKeychain") ||
              !_cups_strcasecmp(line, "ServerRoot") ||
+             !_cups_strcasecmp(line, "SetEnv") ||
              !_cups_strcasecmp(line, "SMBConfigFile") ||
              !_cups_strcasecmp(line, "StateDir") ||
              !_cups_strcasecmp(line, "SystemGroup") ||
@@ -3495,10 +3424,49 @@ read_cupsd_conf(cups_file_t *fp)        /* I - File to read from */
 static int                             /* O - 1 on success, 0 on failure */
 read_cups_files_conf(cups_file_t *fp)  /* I - File to read from */
 {
-  int          linenum;                /* Current line number */
+  int          i,                      /* Looping var */
+               linenum;                /* Current line number */
   char         line[HTTP_MAX_BUFFER],  /* Line from file */
                *value;                 /* Value from line */
   struct group *group;                 /* Group */
+  static const char * const prohibited_env[] =
+  {                                    /* Prohibited environment variables */
+    "APPLE_LANGUAGE",
+    "AUTH_DOMAIN",
+    "AUTH_INFO_REQUIRED",
+    "AUTH_NEGOTIATE",
+    "AUTH_PASSWORD",
+    "AUTH_UID",
+    "AUTH_USERNAME",
+    "CHARSET",
+    "CLASS",
+    "CLASSIFICATION",
+    "CONTENT_TYPE",
+    "CUPS_CACHEDIR",
+    "CUPS_DATADIR",
+    "CUPS_DOCROOT",
+    "CUPS_FILETYPE",
+    "CUPS_FONTPATH",
+    "CUPS_MAX_MESSAGE",
+    "CUPS_REQUESTROOT",
+    "CUPS_SERVERBIN",
+    "CUPS_SERVERROOT",
+    "CUPS_STATEDIR",
+    "DEVICE_URI",
+    "FINAL_CONTENT_TYPE",
+    "HOME",
+    "LANG",
+    "PPD",
+    "PRINTER",
+    "PRINTER_INFO",
+    "PRINTER_LOCATION",
+    "PRINTER_STATE_REASONS",
+    "RIP_CACHE",
+    "SERVER_ADMIN",
+    "SOFTWARE",
+    "TMPDIR",
+    "USER"
+  };
 
 
  /*
@@ -3536,6 +3504,47 @@ read_cups_files_conf(cups_file_t *fp)    /* I - File to read from */
        }
       }
     }
+    else if (!_cups_strcasecmp(line, "PassEnv") && value)
+    {
+     /*
+      * PassEnv variable [... variable]
+      */
+
+      int valuelen;                    /* Length of variable name */
+
+      for (; *value;)
+      {
+        for (valuelen = 0; value[valuelen]; valuelen ++)
+         if (_cups_isspace(value[valuelen]) || value[valuelen] == ',')
+           break;
+
+        if (value[valuelen])
+        {
+         value[valuelen] = '\0';
+         valuelen ++;
+       }
+
+        for (i = 0; i < (int)(sizeof(prohibited_env) / sizeof(prohibited_env[0])); i ++)
+        {
+          if (!strcmp(value, prohibited_env[i]))
+          {
+           cupsdLogMessage(CUPSD_LOG_ERROR, "Environment variable \"%s\" cannot be passed through on line %d of %s.", value, linenum, CupsFilesFile);
+
+           if (FatalErrors & CUPSD_FATAL_CONFIG)
+             return (0);
+           else
+             break;
+          }
+       }
+
+        if (i >= (int)(sizeof(prohibited_env) / sizeof(prohibited_env[0])))
+          cupsdSetEnv(value, NULL);
+
+        for (value += valuelen; *value; value ++)
+         if (!_cups_isspace(*value) || *value != ',')
+           break;
+      }
+    }
     else if (!_cups_strcasecmp(line, "PrintcapFormat") && value)
     {
      /*
@@ -3581,6 +3590,46 @@ read_cups_files_conf(cups_file_t *fp)    /* I - File to read from */
           return (0);
       }
     }
+    else if (!_cups_strcasecmp(line, "SetEnv") && value)
+    {
+     /*
+      * SetEnv variable value
+      */
+
+      char *valueptr;                  /* Pointer to environment variable value */
+
+      for (valueptr = value; *valueptr && !isspace(*valueptr & 255); valueptr ++);
+
+      if (*valueptr)
+      {
+       /*
+        * Found a value...
+       */
+
+        while (isspace(*valueptr & 255))
+         *valueptr++ = '\0';
+
+        for (i = 0; i < (int)(sizeof(prohibited_env) / sizeof(prohibited_env[0])); i ++)
+        {
+          if (!strcmp(value, prohibited_env[i]))
+          {
+           cupsdLogMessage(CUPSD_LOG_ERROR, "Environment variable \"%s\" cannot be set  on line %d of %s.", value, linenum, CupsFilesFile);
+
+           if (FatalErrors & CUPSD_FATAL_CONFIG)
+             return (0);
+           else
+             break;
+          }
+       }
+
+        if (i >= (int)(sizeof(prohibited_env) / sizeof(prohibited_env[0])))
+         cupsdSetEnv(value, valueptr);
+      }
+      else
+        cupsdLogMessage(CUPSD_LOG_ERROR,
+                       "Missing value for SetEnv directive on line %d of %s.",
+                       linenum, ConfigurationFile);
+    }
     else if (!_cups_strcasecmp(line, "SystemGroup") && value)
     {
      /*

diff --git a/scheduler/job.c b/scheduler/job.c
index 61cda44e2fea198e32f4d097d0a0ecdb525d6ade..5ced0b9d1fc22387c4492c6735eeaa08a0bf161c 100644 (file)
--- a/scheduler/job.c
+++ b/scheduler/job.c
@@ -4779,6 +4779,18 @@ start_job(cupsd_job_t     *job,          /* I - Job ID */
   job->profile  = cupsdCreateProfile(job->id, 0);
   job->bprofile = cupsdCreateProfile(job->id, 1);
 
+#ifdef HAVE_SANDBOX_H
+  if ((!job->profile || !job->bprofile) && UseSandboxing && Sandboxing != CUPSD_SANDBOXING_OFF)
+  {
+   /*
+    * Failure to create the sandbox profile means something really bad has
+    * happened and we need to shutdown immediately.
+    */
+
+    return;
+  }
+#endif /* HAVE_SANDBOX_H */
+
  /*
   * Create the status pipes and buffer...
   */

diff --git a/scheduler/process.c b/scheduler/process.c
index b8d49d8f0de3cea61fd05b6a5834dc3bbc39c1a9..3c1c6ba4f9d49fcc3ebb82dd5e446f902ad2e636 100644 (file)
--- a/scheduler/process.c
+++ b/scheduler/process.c
@@ -98,9 +98,13 @@ cupsdCreateProfile(int job_id,               /* I - Job ID or 0 for none */
 
   if ((fp = cupsTempFile2(profile, sizeof(profile))) == NULL)
   {
+   /*
+    * This should never happen, and is fatal when sandboxing is enabled.
+    */
+
     cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdCreateProfile(job_id=%d, allow_networking=%d) = NULL", job_id, allow_networking);
-    cupsdLogMessage(CUPSD_LOG_ERROR, "Unable to create security profile: %s",
-                    strerror(errno));
+    cupsdLogMessage(CUPSD_LOG_EMERG, "Unable to create security profile: %s", strerror(errno));
+    kill(getpid(), SIGTERM);
     return (NULL);
   }
 
@@ -197,10 +201,8 @@ cupsdCreateProfile(int job_id,             /* I - Job ID or 0 for none */
                 " #\"^%s/\""           /* TempDir/... */
                 " #\"^%s$\""           /* CacheDir */
                 " #\"^%s/\""           /* CacheDir/... */
-                " #\"^%s$\""           /* StateDir */
-                " #\"^%s/\""           /* StateDir/... */
                 "))\n",
-                temp, temp, cache, cache, state, state);
+                temp, temp, cache, cache);
   /* Read common folders */
   cupsFilePrintf(fp,
                  "(allow file-read-data file-read-metadata\n"
@@ -242,8 +244,10 @@ cupsdCreateProfile(int job_id,             /* I - Job ID or 0 for none */
                 " #\"^%s/\""           /* ServerBin/... */
                 " #\"^%s$\""           /* ServerRoot */
                 " #\"^%s/\""           /* ServerRoot/... */
+                " #\"^%s$\""           /* StateDir */
+                " #\"^%s/\""           /* StateDir/... */
                 "))\n",
-                request, request, bin, bin, root, root);
+                request, request, bin, bin, root, root, state, state);
   if (Sandboxing == CUPSD_SANDBOXING_RELAXED)
   {
     /* Limited write access to /Library/Printers/... */

diff --git a/scheduler/server.c b/scheduler/server.c
index cecbabe67f82bf54f6d4ad7fc843b38010ee0edf..a4033791b4da1f5533d367ee78156d14cec5a310 100644 (file)
--- a/scheduler/server.c
+++ b/scheduler/server.c
@@ -34,16 +34,28 @@ void
 cupsdStartServer(void)
 {
  /*
-  * Start color management (as needed)...
+  * Create the default security profile...
   */
 
-  cupsdStartColor();
+  DefaultProfile = cupsdCreateProfile(0, 1);
+
+#ifdef HAVE_SANDBOX_H
+  if (!DefaultProfile && UseSandboxing && Sandboxing != CUPSD_SANDBOXING_OFF)
+  {
+   /*
+    * Failure to create the sandbox profile means something really bad has
+    * happened and we need to shutdown immediately.
+    */
+
+    return;
+  }
+#endif /* HAVE_SANDBOX_H */
 
  /*
-  * Create the default security profile...
+  * Start color management (as needed)...
   */
 
-  DefaultProfile = cupsdCreateProfile(0, 1);
+  cupsdStartColor();
 
  /*
   * Startup all the networking stuff...

diff --git a/test/run-stp-tests.sh b/test/run-stp-tests.sh
index 7eb269a674ae35e250c1388004358b7bbc6b85d7..f83bd5d91b601a7c4de8c42f8772fc64d68110c2 100755 (executable)
--- a/test/run-stp-tests.sh
+++ b/test/run-stp-tests.sh
@@ -489,11 +489,6 @@ StrictConformance Yes
 Browsing Off
 Listen localhost:$port
 Listen $BASE/sock
-PassEnv DYLD_LIBRARY_PATH
-PassEnv LD_LIBRARY_PATH
-PassEnv LD_PRELOAD
-PassEnv LOCALEDIR
-PassEnv SHLIB_PATH
 MaxSubscriptions 3
 MaxLogSize 0
 AccessLogLevel actions
@@ -529,6 +524,12 @@ TempDir $BASE/spool/temp
 AccessLog $BASE/log/access_log
 ErrorLog $BASE/log/error_log
 PageLog $BASE/log/page_log
+
+PassEnv DYLD_LIBRARY_PATH
+PassEnv LD_LIBRARY_PATH
+PassEnv LD_PRELOAD
+PassEnv LOCALEDIR
+PassEnv SHLIB_PATH
 EOF
 
 if test $ssltype != 0 -a `uname` = Darwin; then