206 lines
6.6 KiB
Diff
206 lines
6.6 KiB
Diff
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.
|
|
---
|
|
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);
|