/pidgin/main: fe0e01b28407: Fix for TALOS-CAN-0138

Changeset: fe0e01b2840740d9a07acf9a9788ec22e9dd120f
Author:	 Andrew Victor <[email protected]>
Date:	 2016-06-03 13:26 -0500
Branch:	 release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/fe0e01b28407

Description:

Fix for TALOS-CAN-0138

diffstat:

 libpurple/protocols/mxit/chunk.c |  164 ++++++++++++++++++++++++--------------
 1 files changed, 105 insertions(+), 59 deletions(-)

diffs (truncated from 400 to 300 lines):

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, unsigned 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, unsigned 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;
-	unsigned 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;
@@ -453,27 +478,27 @@ size_t mxit_chunk_create_get_avatar( cha
  */
 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 (%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 */
@@ -501,7 +526,7 @@ gboolean mxit_chunk_parse_offer( char* c
  */
 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 (%zu bytes)\n", datalen );
 
@@ -512,19 +537,24 @@ gboolean mxit_chunk_parse_get( char* chu
 		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] */
-	pos += get_int32( &chunkdata[pos], &(getfile->length) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(getfile->length) );
 
 	/* crc [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(getfile->crc) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(getfile->crc) );
+
+	/* check length does not exceed chunked data length */
+	if ( getfile->length > datalen - pos )
+		return FALSE;
 
 	/* file data */
-	getfile->data = &chunkdata[pos];
+	if ( getfile->length > 0 )
+		getfile->data = &chunkdata[pos];
 
 	return TRUE;
 }
@@ -540,23 +570,28 @@ gboolean mxit_chunk_parse_get( char* chu
  */
 gboolean mxit_chunk_parse_splash( char* chunkdata, size_t datalen, struct splash_chunk* splash )
 {
-	int			pos			= 0;
+	size_t pos = 0;
 
 	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_splash (%zu bytes)\n", datalen );
 
 	memset( splash, 0, sizeof( struct splash_chunk ) );
 
+	/* ensure that the chunk size is atleast the minimum size for a "splash screen" chunk */
+	if ( datalen < 6 )
+		return FALSE;
+
 	/* anchor [1 byte] */
-	pos += get_int8( &chunkdata[pos], &(splash->anchor) );
+	pos += get_int8( &chunkdata[pos], datalen - pos, &(splash->anchor) );
 
 	/* time to show [1 byte] */
-	pos += get_int8( &chunkdata[pos], &(splash->showtime) );
+	pos += get_int8( &chunkdata[pos], datalen - pos, &(splash->showtime) );
 
 	/* background color [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(splash->bgcolor) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(splash->bgcolor) );
 
 	/* file data */
-	splash->data = &chunkdata[pos];
+	if ( pos < datalen )
+		splash->data = &chunkdata[pos];
 
 	/* data length */
 	splash->datalen = datalen - pos;
@@ -575,38 +610,44 @@ gboolean mxit_chunk_parse_splash( char* 
  */
 gboolean mxit_chunk_parse_cr( char* chunkdata, size_t datalen, struct cr_chunk* cr )
 {
-	int				pos			= 0;
-	unsigned int	chunklen	= 0;
+	size_t			pos			= 0;
+	unsigned int	chunkslen	= 0;
 
 	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_cr (%zu bytes)\n", datalen );
 
 	memset( cr, 0, sizeof( struct cr_chunk ) );
 
 	/* id [UTF-8] */
-	pos += get_utf8_string( &chunkdata[pos], cr->id, sizeof( cr->id ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, cr->id, sizeof( cr->id ) );
 
 	/* handle [UTF-8] */
-	pos += get_utf8_string( &chunkdata[pos], cr->handle, sizeof( cr->handle ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, cr->handle, sizeof( cr->handle ) );
 
 	/* operation [1 byte] */
-	pos += get_int8( &chunkdata[pos], &(cr->operation) );
+	pos += get_int8( &chunkdata[pos], datalen - pos, &(cr->operation) );
 
-	/* chunk size [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &chunklen );
+	/* total length of all the chunks that are included [4 bytes] */
+	pos += get_int32( &chunkdata[pos], datalen - pos, &chunkslen );
+
+	/* ensure the chunks size does not exceed the data size */
+	if ( pos + chunkslen > datalen )
+		return FALSE;
 
 	/* parse the resource chunks */
-	while ( chunklen > 0 ) {
-		gchar* chunk = &chunkdata[pos];
+	while ( chunkslen >= MXIT_CHUNK_HEADER_SIZE ) {
+		gchar*	chunk		= &chunkdata[pos];
+		guint32	chunksize	= chunk_length( chunk );
 
-		/* start of chunk data */
-		pos += MXIT_CHUNK_HEADER_SIZE;
+		/* check chunk size against length of received data */
+		if ( pos + MXIT_CHUNK_HEADER_SIZE + chunksize > datalen )
+			return FALSE;
 
 		switch ( chunk_type( chunk ) ) {
 			case CP_CHUNK_SPLASH :			/* splash image */
 				{
 					struct splash_chunk* splash = g_new0( struct splash_chunk, 1 );
 
-					if ( mxit_chunk_parse_splash( &chunkdata[pos], chunk_length( chunk ), splash ) )
+					if ( mxit_chunk_parse_splash( chunk_data( chunk ), chunksize, splash ) )
 						cr->resources = g_list_append( cr->resources, splash );
 					else
 						g_free( splash );
@@ -624,8 +665,8 @@ gboolean mxit_chunk_parse_cr( char* chun
 		}
 
 		/* skip over data to next resource chunk */
-		pos += chunk_length( chunk );
-		chunklen -= ( MXIT_CHUNK_HEADER_SIZE + chunk_length( chunk ) );

_______________________________________________
Commits mailing list
[email protected]
https://pidgin.im/cgi-bin/mailman/listinfo/commits