123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208 |
- From 93e60d3df358c0ae6f3dba79e1c9684657683d89 Mon Sep 17 00:00:00 2001
- From: Till Kamppeter <till.kamppeter@gmail.com>
- Date: Wed, 17 May 2023 11:11:29 +0200
- Subject: [PATCH] beh backend: Use execv() instead of system() - CVE-2023-24805
- With execv() command line arguments are passed as separate strings and
- not the full command line in a single string. This prevents arbitrary
- command execution by escaping the quoting of the arguments in a job
- with a forged job title.
- In addition, done the following fixes and improvements:
- - Do not allow '/' in the scheme of the URI (= backend executable
- name), to assure that only backends inside /usr/lib/cups/backend/
- are used.
- - URI must have ':', to split off scheme, otherwise error out.
- - Check return value of snprintf() to create call path for backend, to
- error out on truncation of a too long scheme or on complete failure
- due to a completely odd scheme.
- - Use strncat() instead of strncpy() for getting scheme from URI, the latter
- does not require setting terminating zero byte in case of truncation.
- - Also exclude "." or ".." as scheme, as directories are not valid CUPS
- backends.
- - Do not use fprintf() in sigterm_handler(), to not interfere with a
- fprintf() which could be running in the main process when
- sigterm_handler() is triggered.
- - Use "static volatile int" for global variable job_canceled.
- Upstream: https://github.com/OpenPrinting/cups-filters/commit/93e60d3df358c0ae6f3dba79e1c9684657683d89
- Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
- ---
- backend/beh.c | 107 +++++++++++++++++++++++++++++++++++++++-----------
- 1 file changed, 84 insertions(+), 23 deletions(-)
- diff --git a/backend/beh.c b/backend/beh.c
- index 225fd27d5..8d51235b1 100644
- --- a/backend/beh.c
- +++ b/backend/beh.c
- @@ -22,12 +22,13 @@
- #include "backend-private.h"
- #include <cups/array.h>
- #include <ctype.h>
- +#include <sys/wait.h>
-
- /*
- * Local globals...
- */
-
- -static int job_canceled = 0; /* Set to 1 on SIGTERM */
- +static volatile int job_canceled = 0; /* Set to 1 on SIGTERM */
-
- /*
- * Local functions...
- @@ -213,21 +214,40 @@ call_backend(char *uri, /* I - URI of final destination */
- char **argv, /* I - Command-line arguments */
- char *filename) { /* I - File name of input data */
- const char *cups_serverbin; /* Location of programs */
- + char *backend_argv[8]; /* Arguments for backend */
- char scheme[1024], /* Scheme from URI */
- *ptr, /* Pointer into scheme */
- - cmdline[65536]; /* Backend command line */
- - int retval;
- + backend_path[2048]; /* Backend path */
- + int pid = 0, /* Process ID of backend */
- + wait_pid, /* Process ID from wait() */
- + wait_status, /* Status from child */
- + retval = 0;
- + int bytes;
-
- /*
- * Build the backend command line...
- */
-
- - strncpy(scheme, uri, sizeof(scheme) - 1);
- - if (strlen(uri) > 1023)
- - scheme[1023] = '\0';
- + scheme[0] = '\0';
- + strncat(scheme, uri, sizeof(scheme) - 1);
- if ((ptr = strchr(scheme, ':')) != NULL)
- *ptr = '\0';
- -
- + else {
- + fprintf(stderr,
- + "ERROR: beh: Invalid URI, no colon (':') to mark end of scheme part.\n");
- + exit (CUPS_BACKEND_FAILED);
- + }
- + if (strchr(scheme, '/')) {
- + fprintf(stderr,
- + "ERROR: beh: Invalid URI, scheme contains a slash ('/').\n");
- + exit (CUPS_BACKEND_FAILED);
- + }
- + if (!strcmp(scheme, ".") || !strcmp(scheme, "..")) {
- + fprintf(stderr,
- + "ERROR: beh: Invalid URI, scheme (\"%s\") is a directory.\n",
- + scheme);
- + exit (CUPS_BACKEND_FAILED);
- + }
- if ((cups_serverbin = getenv("CUPS_SERVERBIN")) == NULL)
- cups_serverbin = CUPS_SERVERBIN;
-
- @@ -235,16 +255,29 @@ call_backend(char *uri, /* I - URI of final destination */
- fprintf(stderr,
- "ERROR: beh: Direct output into a file not supported.\n");
- exit (CUPS_BACKEND_FAILED);
- - } else
- - snprintf(cmdline, sizeof(cmdline),
- - "%s/backend/%s '%s' '%s' '%s' '%s' '%s' %s",
- - cups_serverbin, scheme, argv[1], argv[2], argv[3],
- - /* Apply number of copies only if beh was called with a
- - file name and not with the print data in stdin, as
- - backends should handle copies only if they are called
- - with a file name */
- - (argc == 6 ? "1" : argv[4]),
- - argv[5], filename);
- + }
- +
- + backend_argv[0] = uri;
- + backend_argv[1] = argv[1];
- + backend_argv[2] = argv[2];
- + backend_argv[3] = argv[3];
- + /* Apply number of copies only if beh was called with a file name
- + and not with the print data in stdin, as backends should handle
- + copies only if they are called with a file name */
- + backend_argv[4] = (argc == 6 ? "1" : argv[4]);
- + backend_argv[5] = argv[5];
- + backend_argv[6] = filename;
- + backend_argv[7] = NULL;
- +
- + bytes = snprintf(backend_path, sizeof(backend_path),
- + "%s/backend/%s", cups_serverbin, scheme);
- + if (bytes < 0 || bytes >= sizeof(backend_path))
- + {
- + fprintf(stderr,
- + "ERROR: beh: Invalid scheme (\"%s\"), could not determing backend path.\n",
- + scheme);
- + return (CUPS_BACKEND_FAILED);
- + }
-
- /*
- * Overwrite the device URI and run the actual backend...
- @@ -253,18 +286,44 @@ call_backend(char *uri, /* I - URI of final destination */
- setenv("DEVICE_URI", uri, 1);
-
- fprintf(stderr,
- - "DEBUG: beh: Executing backend command line \"%s\"...\n",
- - cmdline);
- + "DEBUG: beh: Executing backend command line \"%s '%s' '%s' '%s' '%s' '%s' %s\"...\n",
- + backend_path, backend_argv[1], backend_argv[2], backend_argv[3],
- + backend_argv[4], backend_argv[5], backend_argv[6]);
- fprintf(stderr,
- "DEBUG: beh: Using device URI: %s\n",
- uri);
-
- - retval = system(cmdline) >> 8;
- + if ((pid = fork()) == 0) {
- + /*
- + * Child comes here...
- + */
- +
- + /* Run the backend */
- + execv(backend_path, backend_argv);
-
- - if (retval == -1)
- fprintf(stderr, "ERROR: Unable to execute backend command line: %s\n",
- strerror(errno));
-
- + exit(1);
- + } else if (pid < 0) {
- + /*
- + * Unable to fork!
- + */
- +
- + return (CUPS_BACKEND_FAILED);
- + }
- +
- + while ((wait_pid = wait(&wait_status)) < 0 && errno == EINTR);
- +
- + if (wait_pid >= 0 && wait_status) {
- + if (WIFEXITED(wait_status))
- + retval = WEXITSTATUS(wait_status);
- + else if (WTERMSIG(wait_status) != SIGTERM)
- + retval = WTERMSIG(wait_status);
- + else
- + retval = 0;
- + }
- +
- return (retval);
- }
-
- @@ -277,8 +336,10 @@ static void
- sigterm_handler(int sig) { /* I - Signal number (unused) */
- (void)sig;
-
- - fprintf(stderr,
- - "DEBUG: beh: Job canceled.\n");
- + const char * const msg = "DEBUG: beh: Job canceled.\n";
- + /* The if() is to eliminate the return value and silence the warning
- + about an unused return value. */
- + if (write(2, msg, strlen(msg)));
-
- if (job_canceled)
- _exit(CUPS_BACKEND_OK);
|