Fri, 03 Jun 2016 13:26:06 -0500
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; }