[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:  closed
Milestone:  2.6.2               |    Component:  QQ    
  Version:  2.6.1               |   Resolution:  fixed 
 Keywords:  qq buffer overflow  |  
--------------------------------+-------------------------------------------
Changes (by markdoliner at pidgin.im):

  * status:  new => closed
  * resolution:  => fixed


Comment:

 (In [65195a427a4340ca3f017d8d052c0b85c1f482f2]):[[BR]]
 The output buffer passed to qq_encrypt needs to be 17 bytes bigger
 than the data you're encrypting, not 16 bytes bigger.  Fixes #10191.
 It's hard to say whether this actually causes problems.  My guess is
 that it does not.

 However, the way the qq protocol plugin constructs the plain text
 buffer to be passed to qq_encrypt is error prone, and the many calls
 to g_newa(guint8, MAX_PACKET_SIZE) are really bad because
 MAX_PACKET_SIZE is 64KB.  This is a ridiculous amount of space to
 request on the stack.  All these qq_put8 qq_put16 qq_put32 qq_putdata
 functions should be changed to insert data into a dynamically
 allocated GString instead of the stack-allocated buffers that they
 use now.  This eliminates the potential for accidentally overwriting
 the end of the buffer.

 And the second g_newa() for the output buffer passed into qq_encrypt()
 should be changed to allocate space on the heap in most places because,
 as previously noted, 64KB is a ridiculous amount of memory to request
 from the stack.

 Heap allocation may be expensive when compared to stack allocation, but
 I feel it's usually worth it to eliminate the possibilty of buffer
 overflow.

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