[PATCH 2/4] Use NULL instead of "offline" for the user status.
felipe.contreras at gmail.com
Tue Jun 24 06:20:13 EDT 2008
On Tue, Jun 24, 2008 at 11:29 AM, Jorge Villaseñor <salinasv at gmail.com> wrote:
> 2008/6/24 Richard Laager <rlaager at wiktel.com>:
>> On Tue, 2008-06-24 at 01:15 -0400, Elliott Sales de Andrade wrote:
>>> so I haven't seen any show-stoppers lately.
>> Does that mean that if we merged your changes, we could ship MSNP15
>> enabled by default?
> I'm testing qulogic's branch HEAD and hope the crash was fixed. I'm
> not really sure, it looks like some memory corruption.
> I think it's safe to get out 2.4.3 as a bugfix release and soon 2.5.0
> as a great featurefull release with msnp15 enabled by default and all
> that new cool shiny features.
That's not a proper fix, but should reduce the possibility of the crash.
Now, I just found the real issue:
(From Masca's log)
(13:44:26) msn: C: SB 370: CAL 2 foo at hotmail.com
(13:44:26) msn: S: SB 370: 217 2
(13:44:26) MSNP14: get payload len:2
See the problem? "212 2" Means error on transaction #2, but somehow
the code is assuming there's a payload following. So what happens is
that it will try to read 2 more bytes from the buffer and save it to
the payload. Since there aren't any more bytes there's a buffer
overflow and the heap gets corrupted.
The offending code comes from command.c:
msn_check_payload_cmd(const char *str)
g_return_val_if_fail(str != NULL, FALSE);
That means that thanks to is_num(str), each and every error command
sent by the server is considered to have a payload, and if it doesn't;
bad stuff happens.
It was introduced in 71ad73bcb02d2d2f7263d02abf77df511c4e8f8c by
nosnilmot on May 27 2007:
Fix & tidy up payload length detection for commands (errors can have
That's complete and utterly wrong, and I don't know how nobody else
noticed after more than one year. Any time an error comes without
payload, bang! And by the way, I don't think I've ever seen an error
with a payload.
By the way, the whole msn_check_payload_cmd introduced by mayuan goes
against my design; the payload_len field should be set by the command
handler, only after processing the command, so that all cases are
This generalization is completely wrong.
But what do I know, you are the ones in charge and I guess arguing
against my code-style is more important than getting me to fix the
code the way I want. Who would want that!
More information about the Devel