Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Nov 2008 17:54:49 -0800
From: Michael Sweet <mike@...ysw.com>
To: Eygene Ryabinkin <rea-sec@...elabs.ru>
CC: oss-security@...ts.openwall.com, 
 "Steven M. Christey" <coley@...re.org>
Subject: Re: CVE request: CUPS DoS via RSS subscriptions

Eygene Ryabinkin wrote:
> Josh, Mike, *, good day.
> 
> Wed, Nov 19, 2008 at 03:14:43PM -0500, Josh Bressers wrote:
>> So from looking at cups 1.3.7 on Fedora 8, here is what I see:
>>
>> (gdb) bt
>> #0  create_subscription (con=0xb88975c0, uri=0xb889ae00) at ipp.c:5858
>> #1  0xb7facba7 in cupsdProcessIPPRequest (con=0xb88975c0) at ipp.c:615
>> #2  0xb7f88bfc in cupsdReadClient (con=0xb88975c0) at client.c:2253
>> #3  0xb7fc0606 in cupsdDoSelect (timeout=1) at select.c:537
>> #4  0xb7f98710 in main (argc=1, argv=0xbfdd6194) at main.c:817
>> (gdb) list
>> 5853        else if (printer)
>> 5854          cupsdLogMessage(CUPSD_LOG_DEBUG,
>> 5855                          "Added subscription %d for printer \"%s\"",
>> 5856                          sub->id, printer->name);
>> 5857        else
>> 5858          cupsdLogMessage(CUPSD_LOG_DEBUG, "Added subscription %d for server",
>> 5859                          sub->id);
>> 5860
>> 5861        sub->interval = interval;
>> 5862        sub->lease    = lease;
>> (gdb) print sub
>> $1 = (cupsd_subscription_t *) 0x0
>>
>> It would appear to be a NULL pointer dereference.  It seems that this call a
>> few lines above the snippet shown above:
>>  sub = cupsdAddSubscription(mask, printer, job, recipient, 0);
>>
>> will return NULL when the hardcoded value of 100 subscriptions is hit.
> 
> Not really hardcoded -- it is settable with the 'MaxSubscriptions'
> directive.  I had just reproduced the bug with CUPS 1.3.9 at FreeBSD.
> MaxSubscriptions was set to 3 to ease the PoC.  Just repeated
> invocations of 'lpr -m <somefile>' were crashing cups daemon
> reproducibly.
> 
> The attached patch fixes the things for me, but perhaps it needs
> some more polishing.  Will try to take a fresh look at this tomorrow.
> 
> Mike, please, take a look at this!

You'll find a much more complete patch already in CUPS svn for both
1.3.x and 1.4.x, along with a new subscription test for the
"make check" target.  I didn't withhold the patch since the browser
attack vector was closed in 1.3.8...

I've attached my 1.3.x patch...

-- 
______________________________________________________________________
Michael Sweet, Easy Software Products           mike at easysw dot com

Index: test/run-stp-tests.sh
===================================================================
--- test/run-stp-tests.sh	(revision 8145)
+++ test/run-stp-tests.sh	(revision 8146)
@@ -307,6 +307,7 @@
 DocumentRoot $root/doc
 RequestRoot /tmp/cups-$user/spool
 TempDir /tmp/cups-$user/spool/temp
+MaxSubscriptions 3
 MaxLogSize 0
 AccessLog /tmp/cups-$user/log/access_log
 ErrorLog /tmp/cups-$user/log/error_log
Index: test/4.4-subscription-ops.test
===================================================================
--- test/4.4-subscription-ops.test	(revision 8145)
+++ test/4.4-subscription-ops.test	(revision 8146)
@@ -116,7 +116,33 @@
 	EXPECT notify-events
 	DISPLAY notify-events
 }
+{
+	# The name of the test...
+	NAME "Check MaxSubscriptions limits"
 
+	# The operation to use
+	OPERATION Create-Printer-Subscription
+	RESOURCE /
+
+	# The attributes to send
+	GROUP operation
+	ATTR charset attributes-charset utf-8
+	ATTR language attributes-natural-language en
+	ATTR uri printer-uri $method://$hostname:$port/printers/Test1
+
+        GROUP subscription
+	ATTR uri notify-recipient-uri testnotify://
+	ATTR keyword notify-events printer-state-changed
+	ATTR integer notify-lease-duration 5
+
+	# What statuses are OK?
+	STATUS client-error-too-many-subscriptions
+
+	# What attributes do we expect?
+	EXPECT attributes-charset
+	EXPECT attributes-natural-language
+}
+
 #
 # End of "$Id$"
 #
Index: scheduler/subscriptions.c
===================================================================
--- scheduler/subscriptions.c	(revision 8145)
+++ scheduler/subscriptions.c	(revision 8146)
@@ -341,9 +341,55 @@
   * Limit the number of subscriptions...
   */
 
-  if (cupsArrayCount(Subscriptions) >= MaxSubscriptions)
+  if (MaxSubscriptions > 0 && cupsArrayCount(Subscriptions) >= MaxSubscriptions)
+  {
+    cupsdLogMessage(CUPSD_LOG_DEBUG,
+                    "cupsdAddSubscription: Reached MaxSubscriptions %d",
+		    MaxSubscriptions);
     return (NULL);
+  }
 
+  if (MaxSubscriptionsPerJob > 0 && job)
+  {
+    int	count;				/* Number of job subscriptions */
+
+    for (temp = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions),
+             count = 0;
+         temp;
+	 temp = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+      if (temp->job == job)
+        count ++;
+
+    if (count >= MaxSubscriptionsPerJob)
+    {
+      cupsdLogMessage(CUPSD_LOG_DEBUG,
+		      "cupsdAddSubscription: Reached MaxSubscriptionsPerJob %d "
+		      "for job #%d", MaxSubscriptionsPerJob, job->id);
+      return (NULL);
+    }
+  }
+
+  if (MaxSubscriptionsPerPrinter > 0 && dest)
+  {
+    int	count;				/* Number of printer subscriptions */
+
+    for (temp = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions),
+             count = 0;
+         temp;
+	 temp = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+      if (temp->dest == dest)
+        count ++;
+
+    if (count >= MaxSubscriptionsPerPrinter)
+    {
+      cupsdLogMessage(CUPSD_LOG_DEBUG,
+		      "cupsdAddSubscription: Reached "
+		      "MaxSubscriptionsPerPrinter %d for %s",
+		      MaxSubscriptionsPerPrinter, dest->name);
+      return (NULL);
+    }
+  }
+
  /*
   * Allocate memory for this subscription...
   */
@@ -758,7 +804,6 @@
       cupsdLogMessage(CUPSD_LOG_ERROR,
                       "Syntax error on line %d of subscriptions.conf.",
 	              linenum);
-      break;
     }
     else if (!strcasecmp(line, "Events"))
     {
Index: scheduler/ipp.c
===================================================================
--- scheduler/ipp.c	(revision 8145)
+++ scheduler/ipp.c	(revision 8146)
@@ -2119,24 +2119,25 @@
     if (mask == CUPSD_EVENT_NONE)
       mask = CUPSD_EVENT_JOB_COMPLETED;
 
-    sub = cupsdAddSubscription(mask, cupsdFindDest(job->dest), job, recipient,
-                               0);
+    if ((sub = cupsdAddSubscription(mask, cupsdFindDest(job->dest), job,
+                                    recipient, 0)) != NULL)
+    {
+      sub->interval = interval;
 
-    sub->interval = interval;
+      cupsdSetString(&sub->owner, job->username);
 
-    cupsdSetString(&sub->owner, job->username);
+      if (user_data)
+      {
+	sub->user_data_len = user_data->values[0].unknown.length;
+	memcpy(sub->user_data, user_data->values[0].unknown.data,
+	       sub->user_data_len);
+      }
 
-    if (user_data)
-    {
-      sub->user_data_len = user_data->values[0].unknown.length;
-      memcpy(sub->user_data, user_data->values[0].unknown.data,
-             sub->user_data_len);
+      ippAddSeparator(con->response);
+      ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
+		    "notify-subscription-id", sub->id);
     }
 
-    ippAddSeparator(con->response);
-    ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
-                  "notify-subscription-id", sub->id);
-
     if (attr)
       attr = attr->next;
   }
@@ -5590,7 +5591,12 @@
     else
       job = NULL;
 
-    sub = cupsdAddSubscription(mask, printer, job, recipient, 0);
+    if ((sub = cupsdAddSubscription(mask, printer, job, recipient, 0)) == NULL)
+    {
+      send_ipp_status(con, IPP_TOO_MANY_SUBSCRIPTIONS,
+		      _("There are too many subscriptions."));
+      return;
+    }
 
     if (job)
       cupsdLogMessage(CUPSD_LOG_DEBUG, "Added subscription %d for job %d",

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ