Blob Blame History Raw
diff -up cups-filters-1.0.35/NEWS.CVE-2014-4337 cups-filters-1.0.35/NEWS
diff -up cups-filters-1.0.35/utils/cups-browsed.c.CVE-2014-4337 cups-filters-1.0.35/utils/cups-browsed.c
--- cups-filters-1.0.35/utils/cups-browsed.c.CVE-2014-4337	2014-10-08 13:33:12.192067445 +0100
+++ cups-filters-1.0.35/utils/cups-browsed.c	2014-10-08 13:54:45.742569979 +0100
@@ -298,6 +298,75 @@ create_local_queue (const char *name,
   return p;
 }
 
+/*
+ * Remove all illegal characters and replace each group of such characters
+ * by a single dash, return a free()-able string.
+ *
+ * mode = 0: Only allow letters, numbers, and dashes, for turning make/model
+ *           info into a valid print queue name or into a string which can
+ *           be supplied as option value in a filter command line without
+ *           need of quoting
+ * mode = 1: Allow also '/', '.', ',', '_', for cleaning up MIME type
+ *           strings (here available Page Description Languages, PDLs) to
+ *           supply them on a filter command line without quoting
+ *
+ * Especially this prevents from arbitrary code execution by interface scripts
+ * generated for print queues to native IPP printers when a malicious IPP
+ * print service with forged PDL and/or make/model info gets broadcasted into
+ * the local network.
+ */
+
+char *                                 /* O - Cleaned string */
+remove_bad_chars(const char *str_orig, /* I - Original string */
+		 int mode)             /* I - 0: Make/Model, queue name */
+                                       /*     1: MIME types/PDLs */
+{
+  int i, j;
+  int havedash = 0;
+  char *str;
+
+  if (str_orig == NULL)
+    return NULL;
+
+  str = strdup(str_orig);
+
+  /* for later str[strlen(str)-1] access */
+  if (strlen(str) < 1)
+    return str;
+
+  for (i = 0, j = 0; i < strlen(str); i++, j++) {
+    if (((str[i] >= 'A') && (str[i] <= 'Z')) ||
+	((str[i] >= 'a') && (str[i] <= 'z')) ||
+	((str[i] >= '0') && (str[i] <= '9')) ||
+	(mode == 1 && (str[i] == '/' || str[i] == '_' ||
+		       str[i] == '.' || str[i] == ','))) {
+      /* Letter or number, keep it */
+      havedash = 0;
+    } else {
+      /* Replace all other characters by a single '-' */
+      if (havedash == 1)
+	j --;
+      else {
+	havedash = 1;
+	str[j] = '-';
+      }
+    }
+  }
+  /* Add terminating zero */
+  str[j] = '\0';
+  /* Cut off trailing dashes */
+  while (str[strlen(str)-1] == '-')
+    str[strlen(str)-1] = '\0';
+
+  /* Cut off leading dashes */
+  i = 0;
+  while (str[i] == '-')
+    ++i;
+
+  /* keep a free()-able string. +1 for trailing \0 */
+  return memmove(str, str + i, strlen(str) - i + 1);
+}
+
 gboolean handle_cups_queues(gpointer unused) {
   remote_printer_t *p;
   http_t *http;
@@ -392,7 +461,7 @@ gboolean handle_cups_queues(gpointer unu
 	    }
 	    if (default_printer_name)
 	      break;
-	  } 
+	  }
 	}
 	if (default_printer_name &&
 	    !strcasecmp(default_printer_name, p->name)) {
@@ -572,19 +641,30 @@ void generate_local_queue(const char *ho
 			  const char *name,
 			  const char *type,
 			  const char *domain) {
-  char *remote_queue, *remote_host;
+  char *remote_queue = NULL, *remote_host = NULL;
   remote_printer_t *p;
-  char *backup_queue_name, *local_queue_name = NULL;
-  cups_dest_t *dests, *dest;
+  char *backup_queue_name = NULL, *local_queue_name = NULL;
+  cups_dest_t *dests = NULL, *dest = NULL;
   int i, num_dests;
-  const char *val;
+  size_t hl = 0;
+  const char *val = NULL;
 
   /* This is a remote CUPS queue, find queue name and host name */
-  remote_queue = resource + 9;
-  remote_host = strdup(host);
-  if (!strcmp(remote_host + strlen(remote_host) - 6, ".local"))
+  if (strncasecmp(resource, "printers/", 9)) {
+    debug_printf("cups-browsed: resource does not begin 'printers/'\n");
+    return;
+  }
+
+  remote_queue = remove_bad_chars(resource + 9, 0);
+  /* Find the remote host name.
+   * Used in constructing backup_queue_name, so need to sanitize.
+   * strdup() is called inside remove_bad_chars() and result is free()-able.
+   */
+  remote_host = remove_bad_chars(host, 1);
+  hl = strlen(remote_host);
+  if (hl > 6 && !strcmp(remote_host + strlen(remote_host) - 6, ".local"))
     remote_host[strlen(remote_host) - 6] = '\0';
-  if (!strcmp(remote_host + strlen(remote_host) - 7, ".local."))
+  if (hl > 7 && !strcmp(remote_host + strlen(remote_host) - 7, ".local."))
     remote_host[strlen(remote_host) - 7] = '\0';
   debug_printf("cups-browsed: Found CUPS queue: %s on host %s.\n",
 	       remote_queue, remote_host);
@@ -592,7 +672,7 @@ void generate_local_queue(const char *ho
   /* Check if there exists already a CUPS queue with the
      requested name Try name@host in such a case and if
      this is also taken, ignore the printer */
-  if ((backup_queue_name = malloc((strlen(remote_queue) + 
+  if ((backup_queue_name = malloc((strlen(remote_queue) +
 				   strlen(remote_host) + 2) *
 				  sizeof(char))) == NULL) {
     debug_printf("cups-browsed: ERROR: Unable to allocate memory.\n");
@@ -639,6 +719,7 @@ void generate_local_queue(const char *ho
 		     local_queue_name);
 	free (backup_queue_name);
 	free (remote_host);
+	free (remote_queue);
 	cupsFreeDests(num_dests, dests);
 	return;
       }
@@ -724,6 +805,7 @@ void generate_local_queue(const char *ho
 
   free (backup_queue_name);
   free (remote_host);
+  free (remote_queue);
 
   if (p)
     debug_printf("cups-browsed: Bonjour IDs: Service name: \"%s\", "
@@ -958,12 +1040,18 @@ found_cups_printer (const char *remote_h
   char local_resource[HTTP_MAX_URI];
   char *c;
 
+  memset(scheme, 0, sizeof(scheme));
+  memset(username, 0, sizeof(username));
+  memset(host, 0, sizeof(host));
+  memset(resource, 0, sizeof(resource));
+  memset(local_resource, 0, sizeof(local_resource));
+
   httpSeparateURI (HTTP_URI_CODING_ALL, uri,
-		   scheme, sizeof(scheme),
-		   username, sizeof(username),
-		   host, sizeof(host),
+		   scheme, sizeof(scheme) - 1,
+		   username, sizeof(username) - 1,
+		   host, sizeof(host) - 1,
 		   &port,
-		   resource, sizeof(resource));
+		   resource, sizeof(resource)- 1);
 
   /* Check this isn't one of our own broadcasts */
   for (iface = cupsArrayFirst (netifs);
@@ -1071,7 +1159,12 @@ process_browse_data (GIOChannel *source,
   char remote_host[256];
   char uri[1024];
   char info[1024];
-  char *c;
+  char *c = NULL, *end = NULL;
+
+  memset(packet, 0, sizeof(packet));
+  memset(remote_host, 0, sizeof(remote_host));
+  memset(uri, 0, sizeof(uri));
+  memset(info, 0, sizeof(info));
 
   srclen = sizeof (srcaddr);
   got = recvfrom (browsesocket, packet, sizeof (packet) - 1, 0,
@@ -1084,7 +1177,7 @@ process_browse_data (GIOChannel *source,
   }
 
   packet[got] = '\0';
-  httpAddrString (&srcaddr, remote_host, sizeof (remote_host));
+  httpAddrString (&srcaddr, remote_host, sizeof (remote_host) - 1);
 
   /* Check this packet is allowed */
   if (!allowed ((struct sockaddr *) &srcaddr)) {
@@ -1102,28 +1195,42 @@ process_browse_data (GIOChannel *source,
   }
 
   info[0] = '\0';
+
+  /* do not read OOB */
+  end = packet + sizeof(packet);
   c = strchr (packet, '\"');
+  if (c >= end)
+     return TRUE;
+
   if (c) {
     /* Skip location field */
-    for (c++; *c != '\"'; c++)
+    for (c++; c < end && *c != '\"'; c++)
       ;
 
+    if (c >= end)
+       return TRUE;
+
     if (*c == '\"') {
-      for (c++; isspace(*c); c++)
+      for (c++; c < end && isspace(*c); c++)
 	;
     }
 
+    if (c >= end)
+      return TRUE;
+
     /* Is there an info field? */
     if (*c == '\"') {
       int i;
       c++;
       for (i = 0;
-	   i < sizeof (info) - 1 && *c != '\"';
+	   i < sizeof (info) - 1 && *c != '\"' && c < end;
 	   i++, c++)
 	info[i] = *c;
       info[i] = '\0';
     }
   }
+  if (c >= end)
+    return TRUE;
 
   found_cups_printer (remote_host, uri, info);
   recheck_timer ();
@@ -1332,7 +1439,7 @@ send_browse_data (gpointer data)
     while (attr && ippGetGroupTag(attr) == IPP_TAG_PRINTER) {
       const char *attrname = ippGetName(attr);
       int value_tag = ippGetValueTag(attr);
-      
+
       if (!strcmp(attrname, "printer-type") &&
 	  value_tag == IPP_TAG_ENUM) {
 	type = ippGetInteger(attr, 0);