[Pidgin] #14297: purple_parse_oncoming() passes NULL pointer to printf()

[Pidgin]

#14297: purple_parse_oncoming() passes NULL pointer to printf()
-------------------+--------------------------------------------------------
 Reporter:  clh    |        Owner:  MarkDoliner
     Type:  patch  |       Status:  new        
Milestone:         |    Component:  AIM        
  Version:  2.8.0  |   Resolution:             
 Keywords:         |  
-------------------+--------------------------------------------------------

Old description:

> In libpurple/protocols/oscar/oscar.c there are two places where a NULL
> pointer can be passed to purple_debug_info() and ultimately a printf
> variant:
>

> {{{
>   1456          message = (info->status && info->status_len > 0)
>   1457                          ?
> oscar_encoding_to_utf8(info->status_encoding, info->status,
> info->status_len)
>   1458                          : NULL;
> }}}
>

> {{{
>   1464                          itmsurl = (info->itmsurl_len > 0) ?
> oscar_encoding_to_utf8(info->itmsurl_encoding, info->itmsurl,
> info->itmsurl_len) : NULL;
>
> }}}
>
> Both itmsurl and message are passed as character pointers to
> purple_debug_info() and ultimately to a printf variant:
>

> {{{
>   1468                  purple_debug_info("oscar", "Activating status
> '%s' for buddy %s, message = '%s', itmsurl = '%s'\n", status_id,
> info->bn, message, itmsurl);
>
>   1471                  purple_debug_info("oscar", "Activating status
> '%s' for buddy %s, message = '%s'\n", status_id, info->bn, message);
>
> }}}
>
> This diff fixes these places:
>
> {{{
> --- oscar.c.old Tue Jun  7 10:44:41 2011
> +++ oscar.c     Wed Jun 15 15:56:22 2011
> @@ -1455,13 +1455,13 @@
>
>         message = (info->status && info->status_len > 0)
>                         ? oscar_encoding_to_utf8(info->status_encoding,
> info->status, info->status_len)
> -                       : NULL;
> +                       : "";
>
>         if (strcmp(status_id, OSCAR_STATUS_ID_AVAILABLE) == 0) {
>                 /* TODO: If itmsurl is NULL, does that mean the URL has
> been
>                    cleared?  Or does it mean the URL should remain
> unchanged? */
>                 if (info->itmsurl != NULL) {
> -                       itmsurl = (info->itmsurl_len > 0) ?
> oscar_encoding_to_utf8(info->itmsurl_encoding, info->itmsurl,
> info->itmsurl_len) : NULL;
> +                       itmsurl = (info->itmsurl_len > 0) ?
> oscar_encoding_to_utf8(info->itmsurl_encoding, info->itmsurl,
> info->itmsurl_len) : "";
>                 } else if (previous_status != NULL &&
> purple_status_is_available(previous_status)) {
>                         itmsurl =
> g_strdup(purple_status_get_attr_string(previous_status, "itmsurl"));
>                 }
>
> }}}

New description:

 In libpurple/protocols/oscar/oscar.c there are two places where a NULL
 pointer can be passed to purple_debug_info() and ultimately a printf
 variant:


 {{{
   1456          message = (info->status && info->status_len > 0)
   1457                          ?
 oscar_encoding_to_utf8(info->status_encoding, info->status,
 info->status_len)
   1458                          : NULL;
 }}}


 {{{
   1464                          itmsurl = (info->itmsurl_len > 0) ?
 oscar_encoding_to_utf8(info->itmsurl_encoding, info->itmsurl,
 info->itmsurl_len) : NULL;

 }}}

 Both itmsurl and message are passed as character pointers to
 purple_debug_info() and ultimately to a printf variant:


 {{{
   1468                  purple_debug_info("oscar", "Activating status '%s'
 for buddy %s, message = '%s', itmsurl = '%s'\n", status_id, info->bn,
 message, itmsurl);

   1471                  purple_debug_info("oscar", "Activating status '%s'
 for buddy %s, message = '%s'\n", status_id, info->bn, message);

 }}}

 This diff fixes these places:

 {{{
 --- libpurple/protocols/oscar/oscar.c.old       Tue Jun  7 10:44:41 2011
 +++ libpurple/protocols/oscar/oscar.c   Wed Jun 15 16:30:19 2011
 @@ -1465,10 +1465,10 @@
                 } else if (previous_status != NULL &&
 purple_status_is_available(previous_status)) {
                         itmsurl =
 g_strdup(purple_status_get_attr_string(previous_status, "itmsurl"));
                 }
 -               purple_debug_info("oscar", "Activating status '%s' for
 buddy %s, message = '%s', itmsurl = '%s'\n", status_id, info->bn, message,
 itmsurl);
 +               purple_debug_info("oscar", "Activating status '%s' for
 buddy %s, message = '%s', itmsurl = '%s'\n", status_id, info->bn, message
 ? message : "", itmsurl ? itmsurl : "");
                 purple_prpl_got_user_status(account, info->bn, status_id,
 "message", message, "itmsurl", itmsurl, NULL);
         } else {
 -               purple_debug_info("oscar", "Activating status '%s' for
 buddy %s, message = '%s'\n", status_id, info->bn, message);
 +               purple_debug_info("oscar", "Activating status '%s' for
 buddy %s, message = '%s'\n", status_id, info->bn, message ? message : "");
                 purple_prpl_got_user_status(account, info->bn, status_id,
 "message", message, NULL);
         }


 }}}

--

Comment(by clh):

 Fixed patch, original version was very broken.

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