From d47f6aec436e0e9df6554436e391471097686ecc Mon Sep 17 00:00:00 2001

From: Michael R Sweet <michael.r.sweet@gmail.com>

Date: Tue, 8 May 2018 15:24:21 -0700

Subject: [PATCH] Fix local privilege escalation to root and sandbox bypasses

 in scheduler (rdar://37836779, rdar://37836995, rdar://37837252,

 rdar://37837581)

---

 man/cups-files.conf.man.in |  10 ++

 man/cupsd.conf.man.in      |   8 --

 scheduler/conf.c           | 201 +++++++++++++++++++++++--------------

 scheduler/job.c            |  12 +++

 scheduler/process.c        |  16 +--

 scheduler/server.c         |  20 +++-

 test/run-stp-tests.sh      |  11 +-

 7 files changed, 179 insertions(+), 99 deletions(-)

diff --git a/man/cups-files.conf.man.in b/man/cups-files.conf.man.in

index 7b96d687d..baf3cb6af 100644

--- 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 3ffc80e42..36c849398 100644

--- 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 67a91e7a6..b51c6060c 100644

--- 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, "

+    if (!_cups_strcasecmp(line, "

     {

      /*

       * <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 61cda44e2..5ced0b9d1 100644

--- 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 b8d49d8f0..3c1c6ba4f 100644

--- 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 cecbabe67..a4033791b 100644

--- 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 7eb269a67..f83bd5d91 100755

--- 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

-- 

2.17.1