[<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
