Fix for TALOS-CAN-0138 release-2.x.y

Fri, 03 Jun 2016 13:26:06 -0500

author
Andrew Victor <andrew.victor@mxit.com>
date
Fri, 03 Jun 2016 13:26:06 -0500
branch
release-2.x.y
changeset 37833
fe0e01b28407
parent 37827
bb5701f8ae79
child 37834
4cfbbc50569c

Fix for TALOS-CAN-0138

libpurple/protocols/mxit/chunk.c file | annotate | diff | comparison | revisions
--- a/libpurple/protocols/mxit/chunk.c	Fri Jun 03 12:51:24 2016 -0500
+++ b/libpurple/protocols/mxit/chunk.c	Fri Jun 03 13:26:06 2016 -0500
@@ -151,11 +151,15 @@
  * 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 @@
  * 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 @@
  * 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 @@
  * 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 @@
  * 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 @@
  * 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 @@
 	}
 
 	/* 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 @@
  */
 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_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 @@
 		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_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_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 @@
 		}
 
 		/* skip over data to next resource chunk */
-		pos += chunk_length( chunk );
-		chunklen -= ( MXIT_CHUNK_HEADER_SIZE + chunk_length( chunk ) );
+		pos += MXIT_CHUNK_HEADER_SIZE + chunksize;
+		chunkslen -= ( MXIT_CHUNK_HEADER_SIZE + chunksize );
 	}
 
 	return TRUE;
@@ -642,7 +683,7 @@
  */
 gboolean mxit_chunk_parse_sendfile( char* chunkdata, size_t datalen, struct sendfile_chunk* sendfile )
 {
-	int				pos		= 0;
+	size_t			pos		= 0;
 	unsigned short	entries	= 0;
 
 	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_sendfile (%zu bytes)\n", datalen );
@@ -650,19 +691,19 @@
 	memset( sendfile, 0, sizeof( struct sendfile_chunk ) );
 
 	/* number of entries [2 bytes] */
-	pos += get_int16( &chunkdata[pos], &entries );
+	pos += get_int16( &chunkdata[pos], datalen - pos, &entries );
 
 	if ( entries < 1 )		/* no data */
 		return FALSE;
 
 	/* contactAddress [UTF-8 string] */
-	pos += get_utf8_string( &chunkdata[pos], sendfile->username, sizeof( sendfile->username ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, sendfile->username, sizeof( sendfile->username ) );
 
 	/* status [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(sendfile->status) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(sendfile->status) );
 
 	/* status message [UTF-8 string] */
-	pos += get_utf8_string( &chunkdata[pos], sendfile->statusmsg, sizeof( sendfile->statusmsg ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, sendfile->statusmsg, sizeof( sendfile->statusmsg ) );
 
 	return TRUE;
 }
@@ -678,7 +719,7 @@
  */
 gboolean mxit_chunk_parse_get_avatar( char* chunkdata, size_t datalen, struct getavatar_chunk* avatar )
 {
-	int				pos			= 0;
+	size_t			pos			= 0;
 	unsigned int	numfiles	= 0;
 
 	purple_debug_info( MXIT_PLUGIN_ID, "mxit_chunk_parse_get_avatar (%zu bytes)\n", datalen );
@@ -686,37 +727,42 @@
 	memset( avatar, 0, sizeof( struct getavatar_chunk ) );
 
 	/* number of files [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &numfiles );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &numfiles );
 
 	if ( numfiles < 1 )		/* no data */
 		return FALSE;
 
 	/* mxitId [UTF-8 string] */
-	pos += get_utf8_string( &chunkdata[pos], avatar->mxitid, sizeof( avatar->mxitid ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, avatar->mxitid, sizeof( avatar->mxitid ) );
 
 	/* avatar id [UTF-8 string] */
-	pos += get_utf8_string( &chunkdata[pos], avatar->avatarid, sizeof( avatar->avatarid ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, avatar->avatarid, sizeof( avatar->avatarid ) );
 
 	/* format [UTF-8 string] */
-	pos += get_utf8_string( &chunkdata[pos], avatar->format, sizeof( avatar->format ) );
+	pos += get_utf8_string( &chunkdata[pos], datalen - pos, avatar->format, sizeof( avatar->format ) );
 
 	/* bit depth [1 byte] */
-	pos += get_int8( &chunkdata[pos], &(avatar->bitdepth) );
+	pos += get_int8( &chunkdata[pos], datalen - pos, &(avatar->bitdepth) );
 
 	/* crc [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(avatar->crc) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(avatar->crc) );
 
 	/* width [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(avatar->width) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(avatar->width) );
 
 	/* height [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(avatar->height) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(avatar->height) );
 
 	/* file length [4 bytes] */
-	pos += get_int32( &chunkdata[pos], &(avatar->length) );
+	pos += get_int32( &chunkdata[pos], datalen - pos, &(avatar->length) );
+
+	/* check length does not exceed chunked data length */
+	if ( avatar->length > datalen - pos )
+		return FALSE;
 
 	/* file data */
-	avatar->data = &chunkdata[pos];
+	if ( avatar->length > 0 )
+		avatar->data = &chunkdata[pos];
 
 	return TRUE;
 }

mercurial