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