From d47f6aec436e0e9df6554436e391471097686ecc Mon Sep 17 00:00:00 2001 From: Michael R Sweet 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 \fR... \fB\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, " @@ -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