0001-beh-backend-Use-execv-instead-of-system-CVE-2023-24805.patch 6.8 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208
  1. From 93e60d3df358c0ae6f3dba79e1c9684657683d89 Mon Sep 17 00:00:00 2001
  2. From: Till Kamppeter <till.kamppeter@gmail.com>
  3. Date: Wed, 17 May 2023 11:11:29 +0200
  4. Subject: [PATCH] beh backend: Use execv() instead of system() - CVE-2023-24805
  5. With execv() command line arguments are passed as separate strings and
  6. not the full command line in a single string. This prevents arbitrary
  7. command execution by escaping the quoting of the arguments in a job
  8. with a forged job title.
  9. In addition, done the following fixes and improvements:
  10. - Do not allow '/' in the scheme of the URI (= backend executable
  11. name), to assure that only backends inside /usr/lib/cups/backend/
  12. are used.
  13. - URI must have ':', to split off scheme, otherwise error out.
  14. - Check return value of snprintf() to create call path for backend, to
  15. error out on truncation of a too long scheme or on complete failure
  16. due to a completely odd scheme.
  17. - Use strncat() instead of strncpy() for getting scheme from URI, the latter
  18. does not require setting terminating zero byte in case of truncation.
  19. - Also exclude "." or ".." as scheme, as directories are not valid CUPS
  20. backends.
  21. - Do not use fprintf() in sigterm_handler(), to not interfere with a
  22. fprintf() which could be running in the main process when
  23. sigterm_handler() is triggered.
  24. - Use "static volatile int" for global variable job_canceled.
  25. Upstream: https://github.com/OpenPrinting/cups-filters/commit/93e60d3df358c0ae6f3dba79e1c9684657683d89
  26. Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
  27. ---
  28. backend/beh.c | 107 +++++++++++++++++++++++++++++++++++++++-----------
  29. 1 file changed, 84 insertions(+), 23 deletions(-)
  30. diff --git a/backend/beh.c b/backend/beh.c
  31. index 225fd27d5..8d51235b1 100644
  32. --- a/backend/beh.c
  33. +++ b/backend/beh.c
  34. @@ -22,12 +22,13 @@
  35. #include "backend-private.h"
  36. #include <cups/array.h>
  37. #include <ctype.h>
  38. +#include <sys/wait.h>
  39. /*
  40. * Local globals...
  41. */
  42. -static int job_canceled = 0; /* Set to 1 on SIGTERM */
  43. +static volatile int job_canceled = 0; /* Set to 1 on SIGTERM */
  44. /*
  45. * Local functions...
  46. @@ -213,21 +214,40 @@ call_backend(char *uri, /* I - URI of final destination */
  47. char **argv, /* I - Command-line arguments */
  48. char *filename) { /* I - File name of input data */
  49. const char *cups_serverbin; /* Location of programs */
  50. + char *backend_argv[8]; /* Arguments for backend */
  51. char scheme[1024], /* Scheme from URI */
  52. *ptr, /* Pointer into scheme */
  53. - cmdline[65536]; /* Backend command line */
  54. - int retval;
  55. + backend_path[2048]; /* Backend path */
  56. + int pid = 0, /* Process ID of backend */
  57. + wait_pid, /* Process ID from wait() */
  58. + wait_status, /* Status from child */
  59. + retval = 0;
  60. + int bytes;
  61. /*
  62. * Build the backend command line...
  63. */
  64. - strncpy(scheme, uri, sizeof(scheme) - 1);
  65. - if (strlen(uri) > 1023)
  66. - scheme[1023] = '\0';
  67. + scheme[0] = '\0';
  68. + strncat(scheme, uri, sizeof(scheme) - 1);
  69. if ((ptr = strchr(scheme, ':')) != NULL)
  70. *ptr = '\0';
  71. -
  72. + else {
  73. + fprintf(stderr,
  74. + "ERROR: beh: Invalid URI, no colon (':') to mark end of scheme part.\n");
  75. + exit (CUPS_BACKEND_FAILED);
  76. + }
  77. + if (strchr(scheme, '/')) {
  78. + fprintf(stderr,
  79. + "ERROR: beh: Invalid URI, scheme contains a slash ('/').\n");
  80. + exit (CUPS_BACKEND_FAILED);
  81. + }
  82. + if (!strcmp(scheme, ".") || !strcmp(scheme, "..")) {
  83. + fprintf(stderr,
  84. + "ERROR: beh: Invalid URI, scheme (\"%s\") is a directory.\n",
  85. + scheme);
  86. + exit (CUPS_BACKEND_FAILED);
  87. + }
  88. if ((cups_serverbin = getenv("CUPS_SERVERBIN")) == NULL)
  89. cups_serverbin = CUPS_SERVERBIN;
  90. @@ -235,16 +255,29 @@ call_backend(char *uri, /* I - URI of final destination */
  91. fprintf(stderr,
  92. "ERROR: beh: Direct output into a file not supported.\n");
  93. exit (CUPS_BACKEND_FAILED);
  94. - } else
  95. - snprintf(cmdline, sizeof(cmdline),
  96. - "%s/backend/%s '%s' '%s' '%s' '%s' '%s' %s",
  97. - cups_serverbin, scheme, argv[1], argv[2], argv[3],
  98. - /* Apply number of copies only if beh was called with a
  99. - file name and not with the print data in stdin, as
  100. - backends should handle copies only if they are called
  101. - with a file name */
  102. - (argc == 6 ? "1" : argv[4]),
  103. - argv[5], filename);
  104. + }
  105. +
  106. + backend_argv[0] = uri;
  107. + backend_argv[1] = argv[1];
  108. + backend_argv[2] = argv[2];
  109. + backend_argv[3] = argv[3];
  110. + /* Apply number of copies only if beh was called with a file name
  111. + and not with the print data in stdin, as backends should handle
  112. + copies only if they are called with a file name */
  113. + backend_argv[4] = (argc == 6 ? "1" : argv[4]);
  114. + backend_argv[5] = argv[5];
  115. + backend_argv[6] = filename;
  116. + backend_argv[7] = NULL;
  117. +
  118. + bytes = snprintf(backend_path, sizeof(backend_path),
  119. + "%s/backend/%s", cups_serverbin, scheme);
  120. + if (bytes < 0 || bytes >= sizeof(backend_path))
  121. + {
  122. + fprintf(stderr,
  123. + "ERROR: beh: Invalid scheme (\"%s\"), could not determing backend path.\n",
  124. + scheme);
  125. + return (CUPS_BACKEND_FAILED);
  126. + }
  127. /*
  128. * Overwrite the device URI and run the actual backend...
  129. @@ -253,18 +286,44 @@ call_backend(char *uri, /* I - URI of final destination */
  130. setenv("DEVICE_URI", uri, 1);
  131. fprintf(stderr,
  132. - "DEBUG: beh: Executing backend command line \"%s\"...\n",
  133. - cmdline);
  134. + "DEBUG: beh: Executing backend command line \"%s '%s' '%s' '%s' '%s' '%s' %s\"...\n",
  135. + backend_path, backend_argv[1], backend_argv[2], backend_argv[3],
  136. + backend_argv[4], backend_argv[5], backend_argv[6]);
  137. fprintf(stderr,
  138. "DEBUG: beh: Using device URI: %s\n",
  139. uri);
  140. - retval = system(cmdline) >> 8;
  141. + if ((pid = fork()) == 0) {
  142. + /*
  143. + * Child comes here...
  144. + */
  145. +
  146. + /* Run the backend */
  147. + execv(backend_path, backend_argv);
  148. - if (retval == -1)
  149. fprintf(stderr, "ERROR: Unable to execute backend command line: %s\n",
  150. strerror(errno));
  151. + exit(1);
  152. + } else if (pid < 0) {
  153. + /*
  154. + * Unable to fork!
  155. + */
  156. +
  157. + return (CUPS_BACKEND_FAILED);
  158. + }
  159. +
  160. + while ((wait_pid = wait(&wait_status)) < 0 && errno == EINTR);
  161. +
  162. + if (wait_pid >= 0 && wait_status) {
  163. + if (WIFEXITED(wait_status))
  164. + retval = WEXITSTATUS(wait_status);
  165. + else if (WTERMSIG(wait_status) != SIGTERM)
  166. + retval = WTERMSIG(wait_status);
  167. + else
  168. + retval = 0;
  169. + }
  170. +
  171. return (retval);
  172. }
  173. @@ -277,8 +336,10 @@ static void
  174. sigterm_handler(int sig) { /* I - Signal number (unused) */
  175. (void)sig;
  176. - fprintf(stderr,
  177. - "DEBUG: beh: Job canceled.\n");
  178. + const char * const msg = "DEBUG: beh: Job canceled.\n";
  179. + /* The if() is to eliminate the return value and silence the warning
  180. + about an unused return value. */
  181. + if (write(2, msg, strlen(msg)));
  182. if (job_canceled)
  183. _exit(CUPS_BACKEND_OK);