[Pidgin] #14295: aim_ssi_item_debug_append() uses NULL string as string pointer to printf()

[Pidgin]

#14295: aim_ssi_item_debug_append() uses NULL string as string pointer to printf()
------------------------------------+---------------------------------------
 Reporter:  clh                     |        Owner:  MarkDoliner
     Type:  patch                   |       Status:  new        
Milestone:  Patches Needing Review  |    Component:  AIM        
  Version:  2.8.0                   |   Resolution:             
 Keywords:                          |  
------------------------------------+---------------------------------------
Changes (by Robby):

  * type:  defect => patch
  * milestone:  => Patches Needing Review


Old description:

> There are are least 2 places in
> libpurple/protocols/oscar/family_feedbag.c where
> aim_ssi_item_debug_append(..,..,item) gets called with an pointer to an
> item with item->name = NULL.
>
> This works on platforms that print (null) in this case, but crashes on
> platforms that don't, like Solaris 10.
>
> One place is intentional:
> static int parsedata(...)
> ...
>   1246                  if ((namelen = byte_stream_get16(bs)))
>   1247                          name = byte_stream_getstr(bs, namelen);
>   1248                  else
>   1249                          name = NULL;
> ...
> Name then gets fed into an item called data
> ...
>   1254                  aim_ssi_item_debug_append(debugstr, "\t",
> aim_ssi_itemlist_add(&od->ssi.official, name, gid, bid, type, data));
>
> After changing this to name = "" instead of name = 0, the client calls
> aim_ssi_item_debug_append() from aim_ssi_sync() with a NULL item->name
> (line 598).
>
> My proposal would be to fix this in aim_ssi_item_debug_append():
> --- family_feedbag.c.orig       Tue Jun  7 10:44:41 2011
> +++ family_feedbag.c    Tue Jun 14 20:42:30 2011
> @@ -100,7 +100,7 @@
>  {
>         g_string_append_printf(str,
>                 "%s gid=0x%04hx, bid=0x%04hx, list_type=0x%04hx [%s],
> name=%s.\n",
> -               prefix, item->gid, item->bid, item->type,
> aim_ssi_type_to_string(item->type), item->name);
> +               prefix, item->gid, item->bid, item->type,
> aim_ssi_type_to_string(item->type), item->name?item->name:"");
>  }

New description:

 There are are least 2 places in libpurple/protocols/oscar/family_feedbag.c
 where aim_ssi_item_debug_append(..,..,item) gets called with an pointer to
 an item with item->name = NULL.

 This works on platforms that print (null) in this case, but crashes on
 platforms that don't, like Solaris 10.

 One place is intentional:
 static int parsedata(...)
 ...

 {{{
   1246                  if ((namelen = byte_stream_get16(bs)))
   1247                          name = byte_stream_getstr(bs, namelen);
   1248                  else
   1249                          name = NULL;
 }}}

 ...
 Name then gets fed into an item called data
 ...

 {{{
   1254                  aim_ssi_item_debug_append(debugstr, "\t",
 aim_ssi_itemlist_add(&od->ssi.official, name, gid, bid, type, data));
 }}}

 After changing this to name = "" instead of name = 0, the client calls
 {{{
 aim_ssi_item_debug_append() from aim_ssi_sync() with a NULL item->name
 (line 598).
 }}}

 My proposal would be to fix this in aim_ssi_item_debug_append():

 {{{
 --- family_feedbag.c.orig       Tue Jun  7 10:44:41 2011
 +++ family_feedbag.c    Tue Jun 14 20:42:30 2011
 @@ -100,7 +100,7 @@
  {
         g_string_append_printf(str,
                 "%s gid=0x%04hx, bid=0x%04hx, list_type=0x%04hx [%s],
 name=%s.\n",
 -               prefix, item->gid, item->bid, item->type,
 aim_ssi_type_to_string(item->type), item->name);
 +               prefix, item->gid, item->bid, item->type,
 aim_ssi_type_to_string(item->type), item->name?item->name:"");
  }
 }}}

--

-- 
Ticket URL: <http://developer.pidgin.im/ticket/14295#comment:1>
Pidgin <http://pidgin.im>
Pidgin