segmentation fault in test/ipptool.c:2557 #4262

Closed
michaelrsweet opened this Issue Jan 21, 2013 · 3 comments

Comments

Projects
None yet
1 participant
Collaborator

michaelrsweet commented Jan 21, 2013

Version: 1.6.1
CUPS.org User: ulrich.windl.rz.uni-regensburg

This patch fixes a segmentation fault in ipptool.c:2557 (cups 1.6.1) when
performing the test 4.2-cups-printer-ops.test.

In that case the IPP printer returned (you can guess the response from the
debug output, "AGT" = Attribute Group Tag):
[0]main_loop:N: accepted connection from 172.20.16.35:52846
[0]main_loop:D: Method: POST
[1]IPP_decode:N: version 0101 op_id 4003 request-id 57953 data size 9293
[3]AGT:1:D: value tag:71
[3]AGT:1:D: name:attributes-charset
[3]AGT:1:D: val:utf-8
[3]AGT:1:D: value tag:72
[3]AGT:1:D: name:attributes-natural-language
[3]AGT:1:D: val:en
[3]AGT:1:D: value tag:69
[3]AGT:1:D: name:printer-uri
[3]AGT:1:D: val:ipp://host.site:1631/printers/Test1
[2]parse_attribute_group:D: group ends with tag:3
[2]parse_attribute_group:D: group ends with tag:3
[1]IPP_decode:D: end of attributes
[1]IPP_decode:D: 9150 bytes of data
[1]IPP_decode:D: end of data
[0]main_loop:E: operation '16387' not implemented
[2]IPP_encode:N: version 0101 status 0501 request-id 57953
[3]build_attribute_group:D: AGT:3
[2]IPP_encode:D: end of data
[0]main_loop:W: failed to get request: Client closed

The original code causes a null-pointer dereference when setting up a variable that will never be used in the loop. The fix just uses some ugly number for group (which shouldn't matter as the loop isn't executed).

diff --git a/test/ipptool.c b/test/ipptool.c
index dac5541..619edee 100644
--- a/test/ipptool.c
+++ b/test/ipptool.c
@@ -2554,7 +2554,8 @@ do_tests(_cups_vars_t vars, / I - Variables */

a = cupsArrayNew((cups_array_func_t)strcmp, NULL);
  • for (attrptr = response->attrs, group = attrptr->group_tag;
  • for (attrptr = response->attrs, group = (attrptr != NULL) ?
  •        attrptr->group_tag : 99999;
     attrptr;
     attrptr = attrptr->next)
    
    {
Collaborator

michaelrsweet commented Jan 21, 2013

CUPS.org User: ulrich.windl.rz.uni-regensburg

In case it's not obvious: The IPP printer returned a 501 error for an operation that is not implemented. It did not send any operation attributes (like charset or language). From RFC 2911 it's not quite clear whether these operation attributes must be sent even when just sending an error without specific messages.
A tool to test IPP compliance should handle incorrect responses more gracefully than with a segmentation fault. The code pattern fixed occcurs at other places, also.

Collaborator

michaelrsweet commented Jan 21, 2013

CUPS.org User: mike

Ulrich,

RFC 2911 does indeed require that a conforming IPP Server respond with attributes-charset and attributes-natural-language as the first two attributes in the response message - see section 3.1.4. So a printer that does not provide any attributes in the response is non-conforming, and there are checks in ipptool for this specifically.

That said, we shouldn't be crashing in this situation. But your suggested fix is not OK since it will cause the attribute group order tests to fail - instead the initial value can be IPP_TAG_ZERO if attrptr is NULL. I've fixed the two spots where this happens.

Collaborator

michaelrsweet commented Jan 21, 2013

"str4262.patch":

Index: test/ipptool.c

--- test/ipptool.c (revision 10826)
+++ test/ipptool.c (working copy)
@@ -2278,7 +2278,8 @@
if (request->attrs)
{
puts("");

  • for (attrptr = request->attrs, group = attrptr->group_tag;

  • for (attrptr = request->attrs,

  •        group = attrptr ? attrptr->group_tag : IPP_TAG_ZERO;
     attrptr;
     attrptr = attrptr->next)
    

    print_attr(attrptr, &group);
    @@ -2632,7 +2633,8 @@

    a = cupsArrayNew((cups_array_func_t)strcmp, NULL);

  • for (attrptr = response->attrs, group = attrptr->group_tag;

  • for (attrptr = response->attrs,

  •        group = attrptr ? attrptr->group_tag : IPP_TAG_ZERO;
     attrptr;
     attrptr = attrptr->next)
    

    {

michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment