Modified version from: https://packages.debian.org/de/sid/cups-filters From: Thorsten Alteholz Date: Fri, 19 May 2023 10:49:35 +0200 Subject: fix CVE-2023-24805 Original patch: https://github.com/OpenPrinting/cups-filters/commit/8f274035756c04efeb77eb654e9d4c4447287d65 From 8f274035756c04efeb77eb654e9d4c4447287d65 Mon Sep 17 00:00:00 2001 From: Till Kamppeter Date: Wed, 17 May 2023 11:12:37 +0200 Subject: [PATCH] Merge pull request from GHSA-gpxc-v2m8-fr3x * 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 forged job title. * beh backend: Extra checks against odd/forged input - CVE-2023-24805 - Do not allow '/' in the scheme of the URI (= backend executable name), to assure that only backends inside /usr/lib/cups/backend/ are used. - Pre-define scheme buffer to empty string, to be defined for case of uri being NULL. - URI must have ':', to split off scheme, otherwise error. - 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. * beh backend: Further improvements - CVE-2023-24805 - 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. --- backend/beh.c | 107 +++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 84 insertions(+), 23 deletions(-) diff --git a/backend/beh.c b/backend/beh.c index 225fd27..8d51235 100644 --- a/backend/beh.c +++ b/backend/beh.c @@ -22,12 +22,13 @@ #include "backend-private.h" #include #include +#include /* * 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);