[Pidgin] #12637: Pdgin crash when accepting a sametime chatroom invitation

[Pidgin]

#12637: Pdgin crash when accepting a sametime chatroom invitation
---------------------------------------------+------------------------------
 Reporter:  angejudor                        |        Owner:  siege   
     Type:  defect                           |       Status:  new     
Milestone:                                   |    Component:  Sametime
  Version:  2.8.0                            |   Resolution:          
 Keywords:  sametime chatroom crash windows  |  
---------------------------------------------+------------------------------

Comment(by jricesterenator):

 Replying to [comment:19 jhkrischel]:
 > Replying to [comment:16 jricesterenator]:
 > > I'm not a C pro either, but I'm not sure that conditional would work.
 > > As background, mString_get(...) grabs the next string element from the
 buffer, advances a position counter, and stores the result in
 m->extraname. So m->extraname won't really exist until you call
 mString_get(b, &m->extraname).
 > >
 >
 > Okay, so when the mwString_get(b, &m->extraname); call is made, there
 seem to be two options -> either b (the buffer with the data) will be
 valid and we'll set &m->extraname, or b will be null, and the
 mwString_get(...) function will hit this line:
 >
 > g_return_if_fail(b != NULL);
 >
 > The way I read that is that if b == NULL, it will return.  mw_str_get()
 returns void, so my guess is that it gracefully fails.
 [[BR]]
 The problem I'm seeing is not if the data runs out, it's when reading a
 message when >1 user is listed (and even the simplest message has at least
 2 users...you and the requestor).

 Here's a simplified example:

 Old-style message:
 {{{
 Header
 user1-placeId
 user1-loginId
 user1-name
 user2-placeId
 user2-loginId
 user2-name
 user3-placeId
 user3-loginId
 user3-name
 }}}

 New-style message:
 {{{
 Header
 user1-placeId
 user1-loginId
 user1-name
 user1-extraname
 user2-placeId
 user2-loginId
 user2-name
 user2-extraname
 user3-placeId
 user3-loginId
 user3-name
 user3-extraname
 }}}

 Now imagine we're running the fixed code and receive an old-style message
 with multiple users listed in it. Sure, the graceful fail would come into
 play after reading user3-name. The problem is that after user1-name,
 there's still 2 more users worth of data. So when we try to parse out the
 missing user1-extraname, it won't fail, but instead it'll grab
 user2-placeId data. And from there on out everything gets offset and is
 reading the wrong data.

 In the simple example, data is just stored in the wrong fields. In a real
 situation, the wrong bytes would be parsed and it'd end up a jumble of
 nothing.

 ||'''Incoming old-style message'''|| '''Parsed as''' ||
 ||Header||Header||
 ||user1-placeId||user1-placeId||
 ||user1-loginId||user1-loginId||
 ||user1-name||user1-name||
 ||user2-placeId||user1-extraname||
 ||user2-loginId||user2-placeId||
 ||user2-name||user2-loginId||
 ||user3-placeId||user2-name||
 ||user3-loginId||user2-extraname||
 ||user3-name||user3-placeId||
 ||xx||user3-loginId (graceful fail, but not data)||
 ||xx||user3-name (graceful fail, but not data)||
 ||xx||user3-extraname (graceful fail, but not data)||

 Really, it'd end up in the same situation as the current crash, excepted
 it'd be a problem for pre-8.5.1 users.

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