Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11863 closed defect (fixed)

Using "Get X-Status Msg" on offline ICQ buddy causes Pidgin's crash

Reported by: Fellrond Owned by: MarkDoliner
Milestone: 2.7.1 Component: ICQ
Version: 2.7.0 Keywords: x-status oscar icq 2.7.0
Cc: Photon, diskin, albert

Description (last modified by Fellrond)

System: Ubuntu 10.04 LTS "Lucid Lynx" Arch: x86-64

How to reproduce:

  1. Start Pidgin
  1. Right-click at offline ICQ buddy in contact list or in chat window header
  1. Choose "Get X-Status Msg"
  1. Get crash instead.

Backtrace attached to this ticket.

Attachments (1)

pidgin-backtrace.log (8.4 KB) - added by Fellrond 9 years ago.
A backtrace to ticket #11863

Download all attachments as: .zip

Change History (35)

Changed 9 years ago by Fellrond

A backtrace to ticket #11863

comment:1 Changed 9 years ago by Robby

  • Milestone set to 2.7.1

comment:2 Changed 9 years ago by Fellrond

  • Description modified (diff)

comment:3 Changed 9 years ago by darkrain42

Ticket #11880 has been marked as a duplicate of this ticket.

comment:4 Changed 9 years ago by darkrain42

Also https://bugzilla.redhat.com/show_bug.cgi?id=592258 & https://bugzilla.redhat.com/show_bug.cgi?id=592194.

The issue is that 8c322571bfb4f2d00c9126e59746445f877be296 updated purple_parse_msgerr, but only updated one of the two possible call sites (the other one being misc.c:generror().

<&nosnilmot> ah ha. and my attempt to reproduce did log this:
 (10:03:07) oscar: icbm error: received response from request without a buddy name!
 so error() is returning 0 and it's falling back to generror() ?
< darkrain42> Yeah, I think so.
< darkrain42> I guess generror() could be taught to add the extra argument if it's 
                       an ICBM SNAC (these names make me feel silly), if there's no better 
                       solution
<&nosnilmot> there's also the question of why the response has no buddy name

comment:5 Changed 9 years ago by nosnilmot

Ticket #11883 has been marked as a duplicate of this ticket.

comment:6 Changed 9 years ago by nosnilmot

it may be sufficient to make error() in family_icbm.c return 1 in the short-cut cases instead of return 0, but I don't know if that will prevent all cases where purple_parse_msgerr() may be called with the wrong number of parameters.

comment:7 Changed 9 years ago by Robby

Ticket #11894 has been marked as a duplicate of this ticket.

comment:8 follow-up: Changed 9 years ago by Photon

Are you sure that it's a duplicate of #11894? I tried to disable showing offline buddies and still get the crash when connecting to ICQ...

comment:9 in reply to: ↑ 8 Changed 9 years ago by nosnilmot

Replying to Photon:

Are you sure that it's a duplicate of #11894? I tried to disable showing offline buddies and still get the crash when connecting to ICQ...

Yes, it's the same backtrace. The summary of this bug does not cover all cases that may trigger it.

comment:10 Changed 9 years ago by Photon

Thanks, got it. :)

comment:11 Changed 9 years ago by darkrain42

Ticket #11877 has been marked as a duplicate of this ticket.

comment:12 Changed 9 years ago by nosnilmot

There are more reports of this against Pidgin in Fedora than I'd like to admit : https://bugzilla.redhat.com/show_bug.cgi?id=548128 (so it's not confined to X-Status support but that certainly made it more prominent)

comment:13 Changed 9 years ago by darkrain42

Ticket #11068 has been marked as a duplicate of this ticket.

comment:14 Changed 9 years ago by darkrain42

Ticket #11909 has been marked as a duplicate of this ticket.

comment:15 follow-up: Changed 9 years ago by alcik

I thik, tah problem is in function oscar_normalize in file oscar.c

7201:   tmp1 = g_utf8_strdown(buf, -1);
7202:   tmp2 = g_utf8_normalize(tmp1, -1, G_NORMALIZE_DEFAULT);
7203:   strcpy(buf, tmp2);

Function g_utf8_normalize may return NULL if string is not valid UTF-8.
And second argument of strcpy could not be NULL

comment:16 in reply to: ↑ 15 Changed 9 years ago by nosnilmot

Replying to alcik:

I thik, tah problem is in function oscar_normalize in file oscar.c

No, that's not the problem. The problem is generror() calls purple_parse_msgerr() with one too few arguments, so the buddy name is read from random memory and that is unlikely to be valid UTF-8, which is why it blows up later on in oscar_normalize.

comment:17 Changed 9 years ago by darkrain42

Ticket #11937 has been marked as a duplicate of this ticket.

comment:18 Changed 9 years ago by darkrain42

Ticket #11947 has been marked as a duplicate of this ticket.

comment:19 Changed 9 years ago by darkrain42

Ticket #11955 has been marked as a duplicate of this ticket.

comment:20 Changed 9 years ago by markdoliner@…

  • Resolution set to fixed
  • Status changed from new to closed

(In e3dd36706068f3b8eabd630ff71d270c145cce42):
If we get an error SNAC on the ICBM family and it's missing buddy name then don't fallthrough to the default error handler in misc.c. This was causing purple_parse_msgerr() in oscar.c to get called with different va_args than it was expecting, which caused a crash. Specifically when trying to fetch the ICQ x-status of an offline buddy.

Fixes #11863. This is nosnilmot's patch, I believe. I had no part in it, other than verifying that I do believe it'll fix the crash.

comment:21 Changed 9 years ago by darkrain42

Ticket #11975 has been marked as a duplicate of this ticket.

comment:22 Changed 9 years ago by darkrain42

Ticket #11982 has been marked as a duplicate of this ticket.

comment:23 Changed 9 years ago by darkrain42

Ticket #11983 has been marked as a duplicate of this ticket.

comment:24 Changed 9 years ago by datallah

Ticket #11985 has been marked as a duplicate of this ticket.

comment:25 Changed 9 years ago by Photon

Maybe you should update the Help page: http://pidgin.im/support/

comment:26 Changed 9 years ago by nosnilmot@…

(In 3985f7dc9b9cd51a4d50d12c5b4bcd623a0c928d):
Update common issues to reflect that #11863 will be fixed in 2.7.1 Refs #11863

comment:27 Changed 9 years ago by Robby

Ticket #11996 has been marked as a duplicate of this ticket.

comment:28 Changed 9 years ago by Robby

Ticket #12011 has been marked as a duplicate of this ticket.

comment:29 Changed 9 years ago by darkrain42

Ticket #12007 has been marked as a duplicate of this ticket.

comment:30 Changed 9 years ago by darkrain42

Ticket #11993 has been marked as a duplicate of this ticket.

comment:31 Changed 9 years ago by darkrain42

Ticket #11934 has been marked as a duplicate of this ticket.

comment:32 Changed 9 years ago by diskin

When will be pidgin-developers PPA (https://launchpad.net/~pidgin-developers/+archive/ppa) updated? Placing 2.7.0 in it broke some(?) people's ICQ.

comment:33 Changed 9 years ago by markdoliner@…

(In 9bf4b7a3def01690cd528dac96909cce2c143fc9):
* Plucked 913a8d73fc8635dab66f0c26561d1ec9fe366b09 (markdoliner@…): Copy the third hunk from 3c30f64efedafc379b6536852bbb3b6ef5f1f6c9 to trunk Somehow this didn't get propagated in bcae499af351044e047d297b8032fa4dca99c147 So recent official ICQ clients display our HTML tags :-( Refs #11863

* Plucked 4e13d24893523b8cf8f1197f69827e562fde3309 (nosnilmot@…): I forgot to update the RPM spec file for building for older releases with older hicolor-icon-theme packages before release. Oops. Refs: #12073

comment:34 Changed 9 years ago by rekkanoryo@…

(In dd496d006f7414efd30774f23f17af433cd38a0e):
* Plucked cfe0e649dda34d9252d40d8f67e445336a247998 (darkrain42@…): yahoo: Fix a few race-condition crashes at login

(if the account disconnects while these fetches are in-progress).

* Plucked 1724c0c7cbf56b08b5454e796b80173ec9aef481 (rekkanoryo@…): Very hackily implement a fallback mechanism in Yahoo, but not for Yahoo Japan because we don't know hosts we can fall back to there yet. This fallback mechanism just blindly connects to scsa.msg.yahoo.com if the HTTP-based CS lookup fails. I guarantee this will break in the future. Refs #11986.

* Plucked 06fcbcb8d044bcd6887415dc87274071bb724596 (rekkanoryo@…): Change the function of the "proxy_ssl" account option to cover regular HTTP requests as well as HTTPS requests. While this does fix the core issue of #11986, there is still more that I need to do to fix his issue. Refs #11986.

* Plucked f08e4f567582f7d28683d043f759a720ee9142e3 (rekkanoryo@…): Make HTTP proxy detection in the yahoo prpls a bit more robust. This should solve some weird proxy related items I couldn't figure out before. Refs #11986.

* Plucked baeaaf7de69d9f99b7545dda73f219292c4dd106 (datallah@…): Attempt to improve handling of HTTP requests on port 80 when there is a HTTP proxy that requires authentication involved. This also has a fix to make sure that we continue to use the same proxy when we are redirected to an alternative URL. I'm tempted to create a purple_proxy_append_proxy_auth_headers(), but that would have to wait until 2.8.0. Refs #11986. Refs #11908.

* Plucked 52fb5cb4cd8795906a7313dd5edde763a4848a3e (qulogic@…): Normalize the remote passport before sending a P2P message. If it's not lowercase, then the switchboard will reject it, causing a whole lot of extra network traffic as Pidgin attempts to resend the same (incorrect) message.

Fixes #11532.

* Plucked e3dd36706068f3b8eabd630ff71d270c145cce42 (markdoliner@…): If we get an error SNAC on the ICBM family and it's missing buddy name then don't fallthrough to the default error handler in misc.c. This was causing purple_parse_msgerr() in oscar.c to get called with different va_args than it was expecting, which caused a crash. Specifically when trying to fetch the ICQ x-status of an offline buddy.

Fixes #11863. This is nosnilmot's patch, I believe. I had no part in it, other than verifying that I do believe it'll fix the crash.

The above revision plucks authorized by Zac West and Robert Vehse.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!