[Pidgin] #10191: qq potential buffer overflow when encrypting a message

[Pidgin]

#10191: qq potential buffer overflow when encrypting a message
--------------------------------+-------------------------------------------
 Reporter:  sagittar            |        Owner:  csyfek
     Type:  defect              |       Status:  new   
Milestone:                      |    Component:  QQ    
  Version:  2.6.1               |   Resolution:        
 Keywords:  qq buffer overflow  |  
--------------------------------+-------------------------------------------
Description changed by sagittar:

Old description:

> currently reading the code for the qq protocol, found the following
> issue:[[BR]]
>
> here's the code that allocates buffer for the encrypted data[[BR]]
> (PROTOCOLS_DIR/qq/qq_network.c)
> {{{
> /* at most 16 bytes more */
> encrypted = g_newa(guint8, data_len + 16);
> encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key);
> }}}
>
> after reading code for ''qq_encrypt'', it seems that the said 16 more
> bytes is not enough(for padding), here's the code[[BR]]
> (PTOTOCOLS_DIR/qq/qq_crypt.c)
> {{{
> padding = (plain_len + 10) % 8;
>         if (padding) {
>                 padding = 8 - padding;
>         }
> ...
> pos++;
> ...
> padding += 2;
> ...
> pos += 7;
> }}}
> the number can be upto 7 + 1 + 2 + 7 = 17. this can be produced by
> sending a text message of 5-bytes long, which results in a
> 71-bytes(''plain_len'') raw plain text according to the qq protocol.
> there's a step that moves this 71-bytes plain text and the padding, which
> is 17 bytes in this case, to the previously allocated buffer. but the
> buffer size is now 87, lacking one more byte. seems we have a buffer
> overflow here. but it seems due to the implementation of ''g_newa'',
> everything goes very well.

New description:

 currently reading code for the qq protocol, found the following
 issue:[[BR]]

 here's the code that allocates buffer for the encrypted data[[BR]]
 (PROTOCOLS_DIR/qq/qq_network.c)
 {{{
 /* at most 16 bytes more */
 encrypted = g_newa(guint8, data_len + 16);
 encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key);
 }}}

 after reading code for ''qq_encrypt'', it seems that the said 16 more
 bytes is not enough(for padding), here's the code[[BR]]
 (PTOTOCOLS_DIR/qq/qq_crypt.c)
 {{{
 padding = (plain_len + 10) % 8;
         if (padding) {
                 padding = 8 - padding;
         }
 ...
 pos++;
 ...
 padding += 2;
 ...
 pos += 7;
 }}}
 the number can be upto 7 + 1 + 2 + 7 = 17. this can be produced by sending
 a text message of 5-bytes long, which results in a 71-bytes(''plain_len'')
 raw plain text according to the qq protocol. there's a step that moves
 this 71-bytes plain text and the padding, which is 17 bytes in this case,
 to the previously allocated buffer. but the buffer size is now 87, lacking
 one more byte. seems we have a buffer overflow here. but it seems due to
 the implementation of ''g_newa'', everything goes very well.

--

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