/pidgin/main: 6d984b02fa91: Merged in rw_grim/pidgin-security (p...

Gary Kramlich grim at reaperworld.com
Mon Jun 20 20:09:59 EDT 2016


Changeset: 6d984b02fa91ff9afa4ef38594b5da6a4479a8cf
Author:	 Gary Kramlich <grim at reaperworld.com>
Date:	 2016-06-12 21:57 -0500
Branch:	 release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/6d984b02fa91

Description:

Merged in rw_grim/pidgin-security (pull request #11)

TALOS-CAN-0120 TALOS-CAN-0135 TALOS-CAN-0138 and TALOS-CAN-0140

diffstat:

 ChangeLog                               |    8 +
 libpurple/protocols/mxit/chunk.c        |  259 +++++++++++++++++++++----------
 libpurple/protocols/mxit/chunk.h        |   79 ++++----
 libpurple/protocols/mxit/filexfer.c     |    6 +-
 libpurple/protocols/mxit/filexfer.h     |    4 +-
 libpurple/protocols/mxit/protocol.c     |  225 +++++++++++---------------
 libpurple/protocols/mxit/protocol.h     |    8 +-
 libpurple/protocols/mxit/splashscreen.c |    2 +-
 libpurple/protocols/mxit/splashscreen.h |    2 +-
 9 files changed, 326 insertions(+), 267 deletions(-)

diffs (truncated from 1189 to 300 lines):

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -13,6 +13,14 @@ version 2.10.13 (MM/DD/YY):
 	Bonjour
 	* Fixed building on Mac OSX (Patrick Cloke) (#16883)
 
+	MXit
+	* Fixed a buffer overflow.  Discovered by Yves Younan of Cisco Talos.
+	  (TALOS-CAN-0120)
+	* Fixed a remote out-of-bounds read.  Discovered by Yves Younan of Cisco
+	  Talos.  (TALOS-CAN-0140)
+	* Fixed a remote out-of-band read.  Discovered by Yves Younan of Cisco
+	  Talos.  (TALOS-CAN-0138, TALOS-CAN-0135)
+
 version 2.10.12 (12/31/15):
 	General:
 	* purple-url-handler now works with Python 3.x (Daniël van Eeden)
diff --git a/libpurple/protocols/mxit/chunk.c b/libpurple/protocols/mxit/chunk.c
--- a/libpurple/protocols/mxit/chunk.c
+++ b/libpurple/protocols/mxit/chunk.c
@@ -151,11 +151,15 @@ static int add_utf8_string( char* chunkd
  * Extract a single byte from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param value			The byte
  *  @return					The number of bytes extracted.
  */
-static int get_int8( const char* chunkdata, char* value )
+static int get_int8( const char* chunkdata, size_t chunklen, char* value )
 {
+	if ( chunklen < sizeof( char ) )
+		return 0;
+
 	*value = *chunkdata;
 
 	return sizeof( char );
@@ -165,11 +169,15 @@ static int get_int8( const char* chunkda
  * Extract a 16-bit value from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param value			The 16-bit value
  *  @return					The number of bytes extracted
  */
-static int get_int16( const char* chunkdata, short* value )
+static int get_int16( const char* chunkdata, size_t chunklen, unsigned short* value )
 {
+	if ( chunklen < sizeof( short ) )
+		return 0;
+
 	*value = ntohs( *( (const short*) chunkdata ) );	/* host byte-order */
 
 	return sizeof( short );
@@ -179,11 +187,15 @@ static int get_int16( const char* chunkd
  * Extract a 32-bit value from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param value			The 32-bit value
  *  @return					The number of bytes extracted
  */
-static int get_int32( const char* chunkdata, int* value )
+static int get_int32( const char* chunkdata, size_t chunklen, unsigned int* value )
 {
+	if ( chunklen < sizeof( int ) )
+		return 0;
+
 	*value = ntohl( *( (const int*) chunkdata ) );	/* host byte-order */
 
 	return sizeof( int );
@@ -194,11 +206,15 @@ static int get_int32( const char* chunkd
  * Extract a 64-bit value from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param value			The 64-bit value
  *  @return					The number of bytes extracted
  */
-static int get_int64( const char* chunkdata, int64_t* value )
+static int get_int64( const char* chunkdata, size_t chunklen, int64_t* value )
 {
+	if ( chunklen < sizeof( int64_t ) )
+		return 0;
+
 	*value = SWAP_64( *( (const int64_t*) chunkdata ) );	/* host byte-order */
 
 	return sizeof( int64_t );
@@ -209,12 +225,16 @@ static int get_int64( const char* chunkd
  * Copy a block of data from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param dest				Where to store the extract data
  *  @param datalen			The length of the data to extract
  *  @return					The number of bytes extracted
  */
-static int get_data( const char* chunkdata, char* dest, int datalen )
+static int get_data( const char* chunkdata, size_t chunklen, char* dest, size_t datalen )
 {
+	if ( chunklen < datalen )
+		return 0;
+
 	memcpy( dest, chunkdata, datalen );
 
 	return datalen;
@@ -224,20 +244,25 @@ static int get_data( const char* chunkda
  * Extract a UTF-8 encoded string from the chunked data.
  *
  *  @param chunkdata		The chunked-data buffer
+ *  @param chunklen			The amount of data available in the buffer.
  *  @param str				A pointer to extracted string.  Must be g_free()'d.
  *  @param maxstrlen		Maximum size of destination buffer.
  *  @return					The number of bytes consumed
  */
-static int get_utf8_string( const char* chunkdata, char* str, int maxstrlen )
+static int get_utf8_string( const char* chunkdata, size_t chunklen, char* str, size_t maxstrlen )
 {
-	int		pos = 0;
-	short	len;
-	int		skip = 0;
+	size_t			pos = 0;
+	unsigned short	len = 0;
+	size_t			skip = 0;
 
 	/* string length [2 bytes] */
-	pos += get_int16( &chunkdata[pos], &len );
+	pos += get_int16( &chunkdata[pos], chunklen - pos, &len );
 
-	if ( len > maxstrlen ) {
+	if ( ( len + pos ) > chunklen ) {
+		/* string length is longer than chunk size */
+		return 0;
+	}
+	else if ( len > maxstrlen ) {
 		/* possible buffer overflow */
 		purple_debug_error( MXIT_PLUGIN_ID, "Buffer overflow detected (get_utf8_string)\n" );
 		skip = len - maxstrlen;
@@ -245,7 +270,7 @@ static int get_utf8_string( const char* 
 	}
 
 	/* string data */
-	pos += get_data( &chunkdata[pos], str, len );
+	pos += get_data( &chunkdata[pos], chunklen - pos, str, len );
 	str[len] = '\0';		/* terminate string */
 
 	return pos + skip;
@@ -263,9 +288,9 @@ static int get_utf8_string( const char* 
  *  @param fileid			A unique ID that identifies this file
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_reject( char* chunkdata, const char* fileid )
+size_t mxit_chunk_create_reject( char* chunkdata, const char* fileid )
 {
-	int		pos		= 0;
+	size_t	pos		= 0;
 
 	/* file id [8 bytes] */
 	pos += add_data( &chunkdata[pos], fileid, MXIT_CHUNK_FILEID_LEN );
@@ -289,9 +314,9 @@ int mxit_chunk_create_reject( char* chun
  *  @param offset			The start offset in the file
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_get( char* chunkdata, const char* fileid, int filesize, int offset )
+size_t mxit_chunk_create_get( char* chunkdata, const char* fileid, size_t filesize, size_t offset )
 {
-	int		pos		= 0;
+	size_t	pos		= 0;
 
 	/* file id [8 bytes] */
 	pos += add_data( &chunkdata[pos], fileid, MXIT_CHUNK_FILEID_LEN );
@@ -314,9 +339,9 @@ int mxit_chunk_create_get( char* chunkda
  *  @param status			The status of the file transfer (see chunk.h)
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_received( char* chunkdata, const char* fileid, unsigned char status )
+size_t mxit_chunk_create_received( char* chunkdata, const char* fileid, unsigned char status )
 {
-	int		pos		= 0;
+	size_t	pos		= 0;
 
 	/* file id [8 bytes] */
 	pos += add_data( &chunkdata[pos], fileid, MXIT_CHUNK_FILEID_LEN );
@@ -338,9 +363,9 @@ int mxit_chunk_create_received( char* ch
  *  @param datalen			The size of the file contents
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_senddirect( char* chunkdata, const char* username, const char* filename, const unsigned char* data, int datalen )
+size_t mxit_chunk_create_senddirect( char* chunkdata, const char* username, const char* filename, const unsigned char* data, size_t datalen )
 {
-	int			pos		= 0;
+	size_t		pos		= 0;
 	const char*	mime	= NULL;
 
 	/* data length [4 bytes] */
@@ -380,10 +405,10 @@ int mxit_chunk_create_senddirect( char* 
  *  @param datalen			The size of the avatar data
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_set_avatar( char* chunkdata, const unsigned char* data, int datalen )
+size_t mxit_chunk_create_set_avatar( char* chunkdata, const unsigned char* data, size_t datalen )
 {
 	char	fileid[MXIT_CHUNK_FILEID_LEN];
-	int			pos = 0;
+	size_t	pos = 0;
 
 	/* id [8 bytes] */
 	memset( &fileid, 0, sizeof( fileid ) );		/* set to 0 for file upload */
@@ -410,9 +435,9 @@ int mxit_chunk_create_set_avatar( char* 
  *  @param avatarId			The Id of the avatar image (as string)
  *  @return					The number of bytes encoded in the buffer
  */
-int mxit_chunk_create_get_avatar( char* chunkdata, const char* mxitId, const char* avatarId )
+size_t mxit_chunk_create_get_avatar( char* chunkdata, const char* mxitId, const char* avatarId )
 {
-	int			pos = 0;
+	size_t	pos = 0;
 
 	/* number of avatars [4 bytes] */
 	pos += add_int32( &chunkdata[pos], 1 );
@@ -449,28 +474,31 @@ int mxit_chunk_create_get_avatar( char* 
  *  @param chunkdata		Chunked data buffer
  *  @param datalen			The length of the chunked data
  *  @param offer			Decoded offerfile information
+ *  @return					TRUE if successfully parsed, otherwise FALSE
  */
-void mxit_chunk_parse_offer( char* chunkdata, int datalen, struct offerfile_chunk* offer )
+gboolean mxit_chunk_parse_offer( char* chunkdata, size_t datalen, struct offerfile_chunk* offer )
 {
-	int			pos			= 0;
+	size_t pos = 0;
 
-	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_offer (%i bytes)\n", datalen );
+	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_offer (%zu bytes)\n", datalen );
+
+	memset( offer, 0, sizeof( struct offerfile_chunk ) );
 
 	/* id [8 bytes] */
-	pos += get_data( &chunkdata[pos], offer->fileid, 8);
+	pos += get_data( &chunkdata[pos], datalen - pos, offer->fileid, 8);
 
 	/* from username [UTF-8] */
-	pos += get_utf8_string( &chunkdata[pos], offer->username, sizeof( offer->username ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, offer->username, sizeof( offer->username ) );
 	mxit_strip_domain( offer->username );
 
 	/* file size [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(offer->filesize) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(offer->filesize) );
 
 	/* filename [UTF-8] */
-	pos += get_utf8_string( &chunkdata[pos], offer->filename, sizeof( offer->filename ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, offer->filename, sizeof( offer->filename ) );
 
 	/* mime type [UTF-8] */
-	pos += get_utf8_string( &chunkdata[pos], offer->mimetype, sizeof( offer->mimetype ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, offer->mimetype, sizeof( offer->mimetype ) );
 
 	/* timestamp [8 bytes] */
 	/* not used by libPurple */
@@ -483,6 +511,8 @@ void mxit_chunk_parse_offer( char* chunk
 
 	/* flags [4 bytes] */
 	/* not used by libPurple */
+
+	return TRUE;
 }
 
 
@@ -492,27 +522,41 @@ void mxit_chunk_parse_offer( char* chunk
  *  @param chunkdata		Chunked data buffer
  *  @param datalen			The length of the chunked data
  *  @param offer			Decoded getfile information
+ *  @return					TRUE if successfully parsed, otherwise FALSE
  */
-void mxit_chunk_parse_get( char* chunkdata, int datalen, struct getfile_chunk* getfile )
+gboolean mxit_chunk_parse_get( char* chunkdata, size_t datalen, struct getfile_chunk* getfile )
 {
-	int			pos			= 0;
+	size_t pos = 0;
 
-	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_file (%i bytes)\n", datalen );
+	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_file (%zu bytes)\n", datalen );
+
+	memset( getfile, 0, sizeof( struct getfile_chunk ) );
+
+	/* ensure that the chunk size is atleast the minimum size for a "get file" chunk */
+	if ( datalen < 20 )
+		return FALSE;
 
 	/* id [8 bytes] */
-	pos += get_data( &chunkdata[pos], getfile->fileid, 8 );
+	pos += get_data( &chunkdata[pos], datalen - pos, getfile->fileid, 8 );
 
 	/* offset [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(getfile->offset) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(getfile->offset) );
 
 	/* file length [4 bytes] */



More information about the Commits mailing list