CVE-2023-24805: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: cups-filters contains backends, filters, and other software required to get the cups printing service working on operating systems other than macos. If you use the Backend Error Handler (beh) to create an accessible network printer, this security vulnerability can cause remote code execution. `beh.c` contains the line `retval = system(cmdline) >> 8;` which calls the `system` command with the operand `cmdline`. `cmdline` contains multiple user controlled, unsanitized values. As a result an attacker with network access to the hosted print server can exploit this vulnerability to inject system commands which are executed in the context of the running server. This issue has been addressed in commit `8f2740357` and is expected to be bundled in the next release. Users are advised to upgrade when possible and to restrict access to network printers in the meantime.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2023-05-17T18:15Z
Repository: https://github.com/OpenPrinting/cups-filters
Language: C
Files changed: backend/beh.c
Commit date: 2023-05-17 11:12:37 +0200
Commit message: 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.
Diff: diff --git a/backend/beh.c b/backend/beh.c
index 7e588c2e3..121e64905 100644
--- a/backend/beh.c
+++ b/backend/beh.c
@@ -24,17 +24,18 @@
 #include <ctype.h>
 #include <stdio.h>
 #include <signal.h>
 #include <string.h>
 #include <errno.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...
 //
@@ -235,63 +236,119 @@ call_backend(char *uri,                 // I - URI of final destination
 	                                //     arguments
 	     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 called CUPS 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,
+                wait_pid,
+                wait_status,
+                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;
 
   if (!strncasecmp(uri, "file:", 5) || uri[0] == '/')
   {
     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];
+  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);
+    exit (CUPS_BACKEND_FAILED);
+  }
 
   //
   // Overwrite the device URI and run the actual backend...
   //
 
   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%s\"...\n",
+	  backend_path, backend_argv[1], backend_argv[2], backend_argv[3],
+	  backend_argv[4], backend_argv[5],
+	  (backend_argv[6] && backend_argv[6][0] ? " " : ""),
+	  (backend_argv[6] && backend_argv[6][0] ? backend_argv[6] : ""));
   fprintf(stderr,
 	  "DEBUG: beh: Using device URI: %s\n",
 	  uri);
 
-  retval = system(cmdline) >> 8;
+  if ((pid = fork()) == 0)
+  {
+    retval = execv(backend_path, backend_argv);
 
-  if (retval == -1)
-    fprintf(stderr, "ERROR: Unable to execute backend command line: %s\n",
-	    strerror(errno));
+    if (retval == -1)
+      fprintf(stderr, "ERROR: Unable to execute backend: %s\n",
+	      strerror(errno));
+    exit (CUPS_BACKEND_FAILED);
+  }
+  else if (pid < 0)
+  {
+    fprintf(stderr, "ERROR: Unable to fork for backend\n");
+    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);
 }
 
 
@@ -302,12 +359,14 @@ call_backend(char *uri,                 // I - URI of final destination
 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);
   else
     job_canceled = 1;

Vulnerable code: static int		job_canceled = 0; // Set to 1 on SIGTERM
		cmdline[65536];		// Backend command line
  int           retval;
  strncpy(scheme, uri, sizeof(scheme) - 1);
  if (strlen(uri) > 1023)
    scheme[1023] = '\0';

  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);
	  "DEBUG: beh: Executing backend command line \"%s\"...\n",
	  cmdline);
  retval = system(cmdline) >> 8;
  if (retval == -1)
    fprintf(stderr, "ERROR: Unable to execute backend command line: %s\n",
	    strerror(errno));
  fprintf(stderr,
	  "DEBUG: beh: Job canceled.\n");
Fixed code: #include <sys/wait.h>
static volatile int	job_canceled = 0; // Set to 1 on SIGTERM
  char          *backend_argv[8];       // Arguments for called CUPS backend
		backend_path[2048];	// Backend path
  int           pid,
                wait_pid,
                wait_status,
                retval = 0;
  int           bytes;

  scheme[0] = '\0';
  strncat(scheme, uri, sizeof(scheme) - 1);
  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);
  }

  backend_argv[0] = uri;
  backend_argv[1] = argv[1];
  backend_argv[2] = argv[2];
  backend_argv[3] = argv[3];
  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);
    exit (CUPS_BACKEND_FAILED);
  }
	  "DEBUG: beh: Executing backend command line \"%s '%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] && backend_argv[6][0] ? " " : ""),
	  (backend_argv[6] && backend_argv[6][0] ? backend_argv[6] : ""));
  if ((pid = fork()) == 0)
  {
    retval = execv(backend_path, backend_argv);
    if (retval == -1)
      fprintf(stderr, "ERROR: Unable to execute backend: %s\n",
	      strerror(errno));
    exit (CUPS_BACKEND_FAILED);
  }
  else if (pid < 0)
  {
    fprintf(stderr, "ERROR: Unable to fork for backend\n");
    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;
  }
  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)));