Re: [LOW] STR #4242: Possible problems found by static analysis of code of cups-1.6.1
Michael Sweet <[email protected]> Thu, 13 Dec 2012 13:08:39 -0800 (PST)
| Newsgroups | gmane.comp.printing.cups.bugs |
|---|---|
| Message-ID | <[email protected]> |
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Closed w/Resolution] 0001: OK 0002: OK 0003: OK 0004: Also the same fix for the mDNSResponder version of http_resolve_cb... 0005: Simpler to compare < 3+sizeof 0006: The values are already type-checked in the big option switch. 0007: This appears to be the correct fix, thanks. 0008: The missing break is on purpose; I've added a comment to this effect... Final patch attached. Link: https://www.cups.org/str.php?L4242 Version: 1.6-current Fix Version: 1.7-current (r10766) _______________________________________________ cups-bugs mailing list [email protected] http://lists.easysw.com/mailman/listinfo/cups-bugs
str4242.patch
(text/plain, 8.3 KB)
Index: backend/usb-libusb.c
===================================================================
--- backend/usb-libusb.c (revision 10756)
+++ backend/usb-libusb.c (working copy)
@@ -672,10 +672,10 @@
* If it didn't exit abort the pending read and wait an additional
* second...
*/
-
+
if (!g.read_thread_done)
{
- fputs("DEBUG: Read thread still active, aborting the pending read...\n",
+ fputs("DEBUG: Read thread still active, aborting the pending read...\n",
stderr);
g.wait_eof = 0;
@@ -683,7 +683,7 @@
gettimeofday(&tv, NULL);
cond_timeout.tv_sec = tv.tv_sec + 1;
cond_timeout.tv_nsec = tv.tv_usec * 1000;
-
+
while (!g.read_thread_done)
{
if (pthread_cond_timedwait(&g.read_thread_cond, &g.read_thread_mutex,
@@ -696,9 +696,6 @@
pthread_mutex_unlock(&g.read_thread_mutex);
}
- if (print_fd)
- close(print_fd);
-
/*
* Close the connection and input file and general clean up...
*/
@@ -758,7 +755,7 @@
*/
if (printer->origconf > 0 && printer->origconf != number2)
{
- fprintf(stderr, "DEBUG: Restoring USB device configuration: %d -> %d\n",
+ fprintf(stderr, "DEBUG: Restoring USB device configuration: %d -> %d\n",
number2, printer->origconf);
if ((errcode = libusb_set_configuration(printer->handle,
printer->origconf)) < 0)
@@ -919,7 +916,7 @@
*/
if (((altptr->bInterfaceClass != LIBUSB_CLASS_PRINTER ||
- altptr->bInterfaceSubClass != 1) &&
+ altptr->bInterfaceSubClass != 1) &&
((printer.quirks & USBLP_QUIRK_BAD_CLASS) == 0)) ||
(altptr->bInterfaceProtocol != 1 && /* Unidirectional */
altptr->bInterfaceProtocol != 2) || /* Bidirectional */
@@ -997,7 +994,7 @@
bEndpointAddress;
}
else
- fprintf(stderr, "DEBUG: Uni-directional USB communication "
+ fprintf(stderr, "DEBUG: Uni-directional USB communication "
"only!\n");
printer.write_endp = confptr->interface[printer.iface].
altsetting[printer.altset].
@@ -1376,7 +1373,7 @@
printer->origconf = current;
- if ((errcode =
+ if ((errcode =
libusb_get_config_descriptor (printer->device, printer->conf, &confptr))
< 0)
{
@@ -1388,7 +1385,7 @@
if (number1 != current)
{
- fprintf(stderr, "DEBUG: Switching USB device configuration: %d -> %d\n",
+ fprintf(stderr, "DEBUG: Switching USB device configuration: %d -> %d\n",
current, number1);
if ((errcode = libusb_set_configuration(printer->handle, number1)) < 0)
{
Index: cups/ipp.c
===================================================================
--- cups/ipp.c (revision 10756)
+++ cups/ipp.c (working copy)
@@ -3239,6 +3239,13 @@
ipp->prev = ipp->current;
attr = ipp->current = ipp_add_attr(ipp, NULL, ipp->curtag, IPP_TAG_ZERO, 1);
+ if (!attr)
+ {
+ _cupsSetHTTPError(HTTP_ERROR);
+ DEBUG_puts("1ippReadIO: unable to allocate attribute.");
+ _cupsBufferRelease((char *)buffer);
+ return (IPP_ERROR);
+ }
DEBUG_printf(("2ippReadIO: membername, ipp->current=%p, ipp->prev=%p",
ipp->current, ipp->prev));
@@ -6364,6 +6371,7 @@
_cupsStrFree(attr->values[0].string.language);
attr->values[0].string.language = NULL;
}
+ /* Fall through to other string values */
case IPP_TAG_TEXT :
case IPP_TAG_NAME :
Index: cups/dest-options.c
===================================================================
--- cups/dest-options.c (revision 10756)
+++ cups/dest-options.c (working copy)
@@ -485,7 +485,7 @@
active = NULL;
}
- if (tries >= 0)
+ if (tries >= 100)
{
DEBUG_puts("1cupsCopyDestConflicts: Unable to resolve after 100 tries.");
have_conflicts = -1;
Index: cups/http-support.c
===================================================================
--- cups/http-support.c (revision 10756)
+++ cups/http-support.c (working copy)
@@ -2064,6 +2064,8 @@
error));
#endif /* DEBUG */
}
+
+ httpAddrFreeList(addrlist);
}
}
@@ -2279,6 +2281,8 @@
error));
#endif /* DEBUG */
}
+
+ httpAddrFreeList(addrlist);
}
}
Index: cups/ipp-support.c
===================================================================
--- cups/ipp-support.c (revision 10756)
+++ cups/ipp-support.c (working copy)
@@ -777,8 +777,8 @@
if (!strcmp(attrname, "document-state") &&
enumvalue >= 3 &&
- enumvalue <= (3 + (int)(sizeof(ipp_document_states) /
- sizeof(ipp_document_states[0]))))
+ enumvalue < (3 + (int)(sizeof(ipp_document_states) /
+ sizeof(ipp_document_states[0]))))
return (ipp_document_states[enumvalue - 3]);
else if (!strcmp(attrname, "finishings") ||
!strcmp(attrname, "finishings-actual") ||
@@ -787,8 +787,8 @@
!strcmp(attrname, "finishings-supported"))
{
if (enumvalue >= 3 &&
- enumvalue <= (3 + (int)(sizeof(ipp_finishings) /
- sizeof(ipp_finishings[0]))))
+ enumvalue < (3 + (int)(sizeof(ipp_finishings) /
+ sizeof(ipp_finishings[0]))))
return (ipp_finishings[enumvalue - 3]);
else if (enumvalue >= 0x40000000 &&
enumvalue <= (0x40000000 + (int)(sizeof(ipp_finishings_vendor) /
@@ -798,8 +798,8 @@
else if ((!strcmp(attrname, "job-collation-type") ||
!strcmp(attrname, "job-collation-type-actual")) &&
enumvalue >= 3 &&
- enumvalue <= (3 + (int)(sizeof(ipp_job_collation_types) /
- sizeof(ipp_job_collation_types[0]))))
+ enumvalue < (3 + (int)(sizeof(ipp_job_collation_types) /
+ sizeof(ipp_job_collation_types[0]))))
return (ipp_job_collation_types[enumvalue - 3]);
else if (!strcmp(attrname, "job-state") &&
enumvalue >= IPP_JOB_PENDING && enumvalue <= IPP_JOB_COMPLETED)
@@ -811,16 +811,16 @@
!strcmp(attrname, "orientation-requested-default") ||
!strcmp(attrname, "orientation-requested-supported")) &&
enumvalue >= 3 &&
- enumvalue <= (3 + (int)(sizeof(ipp_orientation_requesteds) /
- sizeof(ipp_orientation_requesteds[0]))))
+ enumvalue < (3 + (int)(sizeof(ipp_orientation_requesteds) /
+ sizeof(ipp_orientation_requesteds[0]))))
return (ipp_orientation_requesteds[enumvalue - 3]);
else if ((!strcmp(attrname, "print-quality") ||
!strcmp(attrname, "print-quality-actual") ||
!strcmp(attrname, "print-quality-default") ||
!strcmp(attrname, "print-quality-supported")) &&
enumvalue >= 3 &&
- enumvalue <= (3 + (int)(sizeof(ipp_print_qualities) /
- sizeof(ipp_print_qualities[0]))))
+ enumvalue < (3 + (int)(sizeof(ipp_print_qualities) /
+ sizeof(ipp_print_qualities[0]))))
return (ipp_print_qualities[enumvalue - 3]);
else if (!strcmp(attrname, "printer-state") &&
enumvalue >= IPP_PRINTER_IDLE && enumvalue <= IPP_PRINTER_STOPPED)
Index: scheduler/job.c
===================================================================
--- scheduler/job.c (revision 10757)
+++ scheduler/job.c (working copy)
@@ -4294,6 +4294,8 @@
else
unload_job(job);
}
+ else
+ free(job);
}
cupsDirClose(dir);
@@ -4385,7 +4387,7 @@
if (!strcmp(name, "time-at-completed"))
{
- if (JobHistory < INT_MAX)
+ if (JobHistory < INT_MAX && attr)
job->history_time = attr->values[0].integer + JobHistory;
else
job->history_time = INT_MAX;
@@ -4393,7 +4395,7 @@
if (job->history_time < JobHistoryUpdate || !JobHistoryUpdate)
JobHistoryUpdate = job->history_time;
- if (JobFiles < INT_MAX)
+ if (JobFiles < INT_MAX && attr)
job->file_time = attr->values[0].integer + JobFiles;
else
job->file_time = INT_MAX;
Index: scheduler/cupsfilter.c
===================================================================
--- scheduler/cupsfilter.c (revision 10756)
+++ scheduler/cupsfilter.c (working copy)
@@ -409,6 +409,7 @@
if (srctype)
{
+ /* sscanf return value already checked above */
sscanf(srctype, "%15[^/]/%255s", super, type);
if ((src = mimeType(mime, super, type)) == NULL)
{
@@ -426,6 +427,7 @@
return (1);
}
+ /* sscanf return value already checked above */
sscanf(dsttype, "%15[^/]/%255s", super, type);
if (!_cups_strcasecmp(super, "printer"))
dst = printer_type;