cupsd IPP parser memory corruption (CVE-2010-2941) #3648

Closed
michaelrsweet opened this Issue Aug 19, 2010 · 4 comments

Comments

Projects
None yet
1 participant
Collaborator

michaelrsweet commented Aug 19, 2010

Version: 1.4.4
CUPS.org User: thoger

Emmanuel Bouillon reported a problem in cupsd allowing remote attacker to reliably crash the daemon using specially-crafted IPP request. Tim Waugh tracked the crash down to the following problem (quoting Tim's comment from Red Hat bugzilla bug #624438):

-- snip --

The problem is a mismatch between different memory allocators.

In the IPP protocol, an attribute can have multiple values, and each value is typed. In the CUPS data model for this, all values for a given attribute must have the same (or a compatible) type.

String types have values allocated with the StrAlloc allocator, a reference-counting string pool.

'Unknown' types use malloc.

By giving the first value for an attribute a value tag of 56, which does not correspond to any particular value type and so is 'unknown', but which also is accepted as 'compatible' with string types due to the value tag range check, CUPS does not reject the request. The 'unknown' value is allocated using malloc; the remaining values are allocated using StrAlloc.

When the in-memory request is freed, free() is used for all values including those allocated using StrAlloc.

The solution is to use exactly the same conditions when deciding how to allocate the value as when freeing it.

-- snip --

Depending on _cups_sp_item_t layout, this can lead to invalid free in 1.4 or possible use-after-free in 1.3.

Attached is Tim's proposed patch.

Collaborator

michaelrsweet commented Aug 31, 2010

CUPS.org User: mike

Updated (simpler) patch is attached.

Collaborator

michaelrsweet commented Nov 11, 2010

CUPS.org User: mike

Fixed in Subversion repository.

Collaborator

michaelrsweet commented Nov 11, 2010

"cups-value-tag-mismatch.patch":

diff -up cups-1.4.2/cups/ipp.c.value-tag-mismatch cups-1.4.2/cups/ipp.c
--- cups-1.4.2/cups/ipp.c.value-tag-mismatch 2010-08-16 11:36:10.077266864 +0100
+++ cups-1.4.2/cups/ipp.c 2010-08-16 11:50:59.460141883 +0100
@@ -1241,32 +1241,53 @@ ippReadIO(void src, / I - Data

      attr->value_tag = tag;
    }
  •   else if ((value_tag >= IPP_TAG_TEXTLANG &&
    
  •         value_tag <= IPP_TAG_MIMETYPE))
    
  •        {
    
  •   else switch (value_tag)
    
  •   {
     /*
      \* String values can sometimes come across in different
      \* forms; accept sets of differing values...
      */
    
  •     if ((tag < IPP_TAG_TEXTLANG || tag > IPP_TAG_MIMETYPE) &&
    
  •         tag != IPP_TAG_NOVALUE)
    
  •   case IPP_TAG_TEXT :
    
  •   case IPP_TAG_NAME :
    
  •   case IPP_TAG_KEYWORD :
    
  •   case IPP_TAG_URI :
    
  •   case IPP_TAG_URISCHEME :
    
  •   case IPP_TAG_LANGUAGE :
    
  •   case IPP_TAG_MIMETYPE :
    
  •   case IPP_TAG_TEXTLANG :
    
  •   case IPP_TAG_NAMELANG :
    
  •     switch (tag)
      {
    
  •     case IPP_TAG_TEXT :
    
  •     case IPP_TAG_NAME :
    
  •     case IPP_TAG_KEYWORD :
    
  •     case IPP_TAG_URI :
    
  •     case IPP_TAG_URISCHEME :
    
  •     case IPP_TAG_LANGUAGE :
    
  •     case IPP_TAG_MIMETYPE :
    
  •     case IPP_TAG_TEXTLANG :
    
  •     case IPP_TAG_NAMELANG :
    
  •     case IPP_TAG_NOVALUE:
    
  •   break;
    
  •     default:
    DEBUG_printf(("1ippReadIO: 1setOf value tag %x(%s) != %x(%s)",
              value_tag, ippTagString(value_tag), tag,
              ippTagString(tag)));
    ipp_buffer_release(buffer);
        return (IPP_ERROR);
      }
    
  •        }
    
  •   else if (value_tag != tag)
    
  •   {
    
  •     DEBUG_printf(("1ippReadIO: value tag %x(%s) != %x(%s)",
    
  •                   value_tag, ippTagString(value_tag), tag,
    
  •           ippTagString(tag)));
    
  •     ipp_buffer_release(buffer);
    
  •     return (IPP_ERROR);
    
  •        }
    
  •   default:
    
  •     if (value_tag != tag)
    
  •     {
    
  •   DEBUG_printf(("1ippReadIO: value tag %x(%s) != %x(%s)",
    
  •             value_tag, ippTagString(value_tag), tag,
    
  •             ippTagString(tag)));
    
  •   ipp_buffer_release(buffer);
    
  •   return (IPP_ERROR);
    
  •     }
    
  •   }
    
        /*
    * Finally, reallocate the attribute array as needed...
    
Collaborator

michaelrsweet commented Nov 11, 2010

"str3648.patch":

Index: CHANGES-1.4.txt

--- CHANGES-1.4.txt (revision 9283)
+++ CHANGES-1.4.txt (working copy)
@@ -5,6 +5,9 @@

- Documentation fixes (STR #3542, STR #3650)
- Localization fixes (STR #3635, STR #3636, STR #3647)
    • cupsAdminSetServerSettings duplicated Listen and Order lines
  • (STR #3645)
    
    • Added DeviceN colorspace support to the CUPS Raster format (STR #3419)

    • ppdMarkDefaults() did not clear the marked field of the previous
      choices (STR #3642)

    • The serial backend would not allow a raw job to be canceled

      Index: cups/ipp.c

      --- cups/ipp.c (revision 9283)
      +++ cups/ipp.c (working copy)
      @@ -1271,7 +1271,9 @@

      attr->value_tag = tag;
      }

  •   else if ((value_tag >= IPP_TAG_TEXTLANG &&
    
  •   else if (value_tag == IPP_TAG_TEXTLANG ||
    
  •            value_tag == IPP_TAG_NAMELANG ||
    
  •        (value_tag >= IPP_TAG_TEXT &&
          value_tag <= IPP_TAG_MIMETYPE))
         {
     /*
    

    @@ -1279,8 +1281,9 @@
    * forms; accept sets of differing values...
    */

  •     if ((tag < IPP_TAG_TEXTLANG || tag > IPP_TAG_MIMETYPE) &&
    
  •         tag != IPP_TAG_NOVALUE)
    
  •     if (tag != IPP_TAG_TEXTLANG && tag != IPP_TAG_NAMELANG &&
    
  •         (tag < IPP_TAG_TEXT || tag > IPP_TAG_MIMETYPE) &&
    
  •     tag != IPP_TAG_NOVALUE)
      {
    DEBUG_printf(("1ippReadIO: 1setOf value tag %x(%s) != %x(%s)",
              value_tag, ippTagString(value_tag), tag,
    

    @@ -2762,6 +2765,7 @@
    {
    case IPP_TAG_TEXT :
    case IPP_TAG_NAME :

  • case IPP_TAG_RESERVED_STRING :
    case IPP_TAG_KEYWORD :
    case IPP_TAG_URI :
    case IPP_TAG_URISCHEME :
    Index: cups/ipp.h
    ===================================================================
    --- cups/ipp.h (revision 9283)
    +++ cups/ipp.h (working copy)
    @@ -93,7 +93,8 @@
    IPP_TAG_END_COLLECTION, /* End of collection value /
    IPP_TAG_TEXT = 0x41, /
    Text value /
    IPP_TAG_NAME, /
    Name value */

  • IPP_TAG_KEYWORD = 0x44, /* Keyword value */

  • IPP_TAG_RESERVED_STRING, /* Reserved for future string value @Private@ */

  • IPP_TAG_KEYWORD, /* Keyword value /
    IPP_TAG_URI, /
    URI value /
    IPP_TAG_URISCHEME, /
    URI scheme value /
    IPP_TAG_CHARSET, /
    Character set value */

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