diff -r -U3 bmp-0.9.7.orig/Input/mpg123/id3_frame_content.c bmp-0.9.7/Input/mpg123/id3_frame_content.c --- bmp-0.9.7.orig/Input/mpg123/id3_frame_content.c 2005-02-25 21:45:03.000000000 -0500 +++ bmp-0.9.7/Input/mpg123/id3_frame_content.c 2005-02-25 21:56:14.000000000 -0500 @@ -44,9 +44,7 @@ char * id3_get_content(struct id3_frame *frame) { - char *text, *text_beg, *ptr; - char buffer[256]; - int spc = sizeof(buffer) - 1; + gchar *text, *text_it; /* Type check */ if (frame->fr_desc->fd_id != ID3_TCON) @@ -56,83 +54,82 @@ if (id3_decompress_frame(frame) == -1) return NULL; - if (*(guint8 *) frame->fr_data == ID3_ENCODING_ISO_8859_1) - text_beg = text = g_strdup((char *) frame->fr_data + 1); - else - text_beg = text = id3_utf16_to_ascii((char *) frame->fr_data + 1); - - /* - * If content is just plain text, return it. - */ - if (text[0] != '(') { - return text; - } - + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, NULL); + + text = text_it = id3_string_decode(encoding, cursor, length); + + if (text == NULL) + return NULL; + /* * Expand ID3v1 genre numbers. */ - ptr = buffer; - while (text[0] == '(' && text[1] != '(' && spc > 0) { - const char *genre; - int num = 0; - - if (text[1] == 'R' && text[2] == 'X') { - text += 4; - genre = _(" (Remix)"); - if (ptr == buffer) - genre++; - + while ((text_it = strstr(text_it, "(")) != NULL) + { + gchar* replace = NULL; + gchar* ref_start = text_it + 1; + + if (*ref_start == ')') + { + /* False alarm */ + ++text_it; + continue; } - else if (text[1] == 'C' && text[2] == 'R') { - text += 4; - genre = _(" (Cover)"); - if (ptr == buffer) - genre++; - + + gsize ref_size = strstr(ref_start, ")") - ref_start; + + if (strncmp(ref_start, "RX", ref_size) == 0) + { + replace = _("Remix"); } - else { - /* Get ID3v1 genre number */ - text++; - while (*text != ')') { - num *= 10; - num += *text++ - '0'; + else if (strncmp(ref_start, "CR", ref_size) == 0) + { + replace = _("Cover"); + } + else + { + /* Maybe an ID3v1 genre? */ + int genre_number; + gchar* genre_number_str = g_strndup(ref_start, ref_size); + if (sscanf(genre_number_str, "%d", &genre_number) > 0) + { + /* Boundary check */ + if (genre_number >= sizeof(mpg123_id3_genres) / sizeof(char *)) + continue; + replace = gettext(mpg123_id3_genres[genre_number]); } - text++; - - /* Boundary check */ - if (num >= sizeof(mpg123_id3_genres) / sizeof(char *)) - continue; - - genre = gettext(mpg123_id3_genres[num]); - - if (ptr != buffer && spc-- > 0) - *ptr++ = '/'; } - /* Expand string into buffer */ - while (*genre != '\0' && spc > 0) { - *ptr++ = *genre++; - spc--; + if (replace != NULL) + { + /* Amazingly hairy code to replace a part of the original genre string + with 'replace'. */ + gchar* copy = g_malloc(strlen(text) - ref_size + strlen(replace) + 1); + gsize pos = 0; + gsize copy_size; + + /* Copy the part before the replaced part */ + copy_size = ref_start - text; + memcpy(copy + pos, text, copy_size); + pos += copy_size; + /* Copy the replacement instead of the original reference */ + copy_size = strlen(replace); + memcpy(copy + pos, replace, copy_size); + pos += copy_size; + /* Copy the rest, including the null */ + memcpy(copy + pos, ref_start + ref_size, strlen(ref_start + ref_size)+1); + + /* Put into original variables */ + gsize offset = text_it - text; + g_free(text); + text = copy; + text_it = text + offset; } + + ++text_it; } - - /* - * Add plaintext refinement. - */ - if (*text == '(') - text++; - if (*text != '\0' && ptr != buffer && spc-- > 0) - *ptr++ = ' '; - while (*text != '\0' && spc > 0) { - *ptr++ = *text++; - spc--; - } - *ptr = '\0'; - - g_free(text_beg); - - /* - * Return the expanded content string. - */ - return g_strdup(buffer); + return text; } diff -r -U3 bmp-0.9.7.orig/Input/mpg123/id3_frame_text.c bmp-0.9.7/Input/mpg123/id3_frame_text.c --- bmp-0.9.7.orig/Input/mpg123/id3_frame_text.c 2005-02-25 21:45:03.000000000 -0500 +++ bmp-0.9.7/Input/mpg123/id3_frame_text.c 2005-02-26 01:25:45.613011872 -0500 @@ -35,21 +35,6 @@ #include "id3_header.h" -char * -id3_utf16_to_ascii(void *utf16) -{ - char ascii[256]; - char *uc = (char *) utf16 + 2; - int i; - - for (i = 0; *uc != 0 && i < sizeof(ascii); i++, uc += 2) - ascii[i] = *uc; - - ascii[i] = 0; - return g_strdup(ascii); -} - - /* * Function id3_get_encoding (frame) * @@ -78,7 +63,12 @@ if (id3_decompress_frame(frame) == -1) return -1; - return *(gint8 *) frame->fr_data; + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, -1); + + return encoding; } @@ -119,6 +109,108 @@ return 0; } +/* Get size of string in bytes including null. */ +gsize id3_string_size(guint8 encoding, const void* text, gsize max_size) +{ + switch ( encoding ) { + case ID3_ENCODING_ISO_8859_1: + case ID3_ENCODING_UTF8: + { + const guint8* text8 = text; + while ( (max_size >= sizeof(*text8)) && (*text8 != 0) ) + { + ++text8; + max_size -= sizeof(*text8); + } + + if (max_size >= sizeof(*text8)) + { + ++text8; + max_size -= sizeof(*text8); + } + + return text8 - (guint8*)text; + } + case ID3_ENCODING_UTF16: + case ID3_ENCODING_UTF16BE: + { + const guint16* text16 = (guint16*)text; + while ( (max_size > 0) && (*text16 != 0) ) + { + ++text16; + max_size -= sizeof(*text16); + } + + if (max_size > 0) + { + ++text16; + max_size -= sizeof(*text16); + } + + return (guint8*)text16 - (guint8*)text; + } + default: + return 0; + } +} + +/* + * Returns a newly-allocated UTF-8 string in the locale's encoding. + * max_size specifies the maximum size of 'text', including terminating nulls. + */ +gchar* id3_string_decode(guint8 encoding, const void* text, gsize max_size) +{ + /* Otherwise, we'll end up passing -1 to functions, eliminating safety benefits. */ + if (max_size <= 0) + return NULL; + + switch( encoding ) + { + case ID3_ENCODING_ISO_8859_1: + { + return g_locale_to_utf8((const gchar*)text, max_size, NULL, NULL, NULL); + } + case ID3_ENCODING_UTF8: + { + gchar* end_text = (gchar*)text; + if (g_utf8_validate((const gchar*)text, max_size, &end_text)) + return g_strndup((const gchar*)text, max_size); + else + { + /* g_utf8_validate() with second argument positive + will return 0 if the string contains nulls. Frequently + someone will pass us a valid null-terminated utf8 + string, with max_size overestimated a bit; so we need + to run another test to truly check validity */ + if((end_text != text) && (*end_text == NULL) && + g_utf8_validate((const gchar*)text, NULL, NULL)) + return g_strndup((const gchar*)text, max_size); + + return NULL; + } + } + case ID3_ENCODING_UTF16: + { + gsize size_bytes = id3_string_size(encoding, text, max_size); + gchar* utf8 = g_convert((const gchar*)text, size_bytes, "UTF-8", "UTF-16", NULL, NULL, NULL); + /* If conversion passes on the BOM as-is, we strip it. */ + if (g_utf8_get_char(utf8) == 0xfeff) + { + gchar* new_utf8 = g_strdup(utf8+3); + g_free(utf8); + utf8 = new_utf8; + } + return utf8; + } + case ID3_ENCODING_UTF16BE: + { + gsize size_bytes = id3_string_size(encoding, text, max_size); + return g_convert((const gchar*)text, size_bytes, "UTF-8", "UTF-16BE", NULL, NULL, NULL); + } + default: + return NULL; + } +} /* * Function id3_get_text (frame) @@ -137,38 +229,21 @@ if (id3_decompress_frame(frame) == -1) return NULL; + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, NULL); + if (frame->fr_desc->fd_id == ID3_TXXX || frame->fr_desc->fd_id == ID3_COMM) { /* * This is a user defined text frame. Skip the description. */ - switch (*(guint8 *) frame->fr_data) { - case ID3_ENCODING_ISO_8859_1: - { - char *text = (char *) frame->fr_data + 1; - - while (*text != 0) - text++; - - return g_strdup(++text); - } - case ID3_ENCODING_UTF16: - { - char *text16 = (char *) frame->fr_data + 1; - - while (*text16 != 0 || *(text16 + 1) != 0) - text16 += 2; - - return id3_utf16_to_ascii(text16 + 2); - } - default: - return NULL; - } + gsize bytes = id3_string_size(encoding, cursor, length); + cursor += bytes; + length -= bytes; } - if (*(guint8 *) frame->fr_data == ID3_ENCODING_ISO_8859_1) - return g_strdup((char *) frame->fr_data + 1); - else - return id3_utf16_to_ascii(((char *) frame->fr_data + 1)); + return id3_string_decode(encoding, cursor, length); } @@ -193,10 +268,12 @@ if (id3_decompress_frame(frame) == -1) return NULL; - if (*(guint8 *) frame->fr_data == ID3_ENCODING_ISO_8859_1) - return g_strdup((char *) frame->fr_data + 1); - else - return id3_utf16_to_ascii((char *) frame->fr_data + 1); + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, NULL); + + return id3_string_decode(encoding, cursor, length); } @@ -216,41 +293,19 @@ if (id3_decompress_frame(frame) == -1) return -1; - /* - * Generate integer according to encoding. - */ - switch (*(guint8 *) frame->fr_data) { - case ID3_ENCODING_ISO_8859_1: - { - char *text = ((char *) frame->fr_data) + 1; - - while (*text >= '0' && *text <= '9') { - number *= 10; - number += *text - '0'; - text++; - } - - return number; - } - case ID3_ENCODING_UTF16: - { - char *text = ((char *) frame->fr_data) + 3; - -/* if (*(gint16 *) frame->fr_data == 0xfeff) */ -/* text++; */ - - while (*text >= '0' && *text <= '9') { - number *= 10; - number += *text - '0'; - text++; - } - - return number; - } - - default: - return -1; - } + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, number); + + gchar* number_str = id3_string_decode(encoding, cursor, length); + if (number_str != NULL) + { + sscanf(number_str, "%d", &number); + g_free(number_str); + } + + return number; } @@ -306,7 +361,6 @@ id3_set_text_number(struct id3_frame *frame, int number) { char buf[64]; - int pos; char *text; /* Type check */ @@ -321,20 +375,12 @@ /* * Create a string with a reversed number. */ - pos = 0; - while (number > 0 && pos < 64) { - buf[pos++] = (number % 10) + '0'; - number /= 10; - } - if (pos == 64) - return -1; - if (pos == 0) - buf[pos++] = '0'; + snprintf(buf, sizeof(buf), "%d", number); /* * Allocate memory for new data. */ - frame->fr_raw_size = pos + 1; + frame->fr_raw_size = strlen(buf) + 1; frame->fr_raw_data = g_malloc(frame->fr_raw_size + 1); /* @@ -342,9 +388,7 @@ */ *(gint8 *) frame->fr_raw_data = ID3_ENCODING_ISO_8859_1; text = (char *) frame->fr_raw_data + 1; - while (--pos >= 0) - *text++ = buf[pos]; - *text = '\0'; + strcpy(text, buf); frame->fr_altered = 1; frame->fr_owner->id3_altered = 1; diff -r -U3 bmp-0.9.7.orig/Input/mpg123/id3_frame_url.c bmp-0.9.7/Input/mpg123/id3_frame_url.c --- bmp-0.9.7.orig/Input/mpg123/id3_frame_url.c 2005-02-25 21:45:03.000000000 -0500 +++ bmp-0.9.7/Input/mpg123/id3_frame_url.c 2005-02-25 22:15:28.000000000 -0500 @@ -48,36 +48,26 @@ /* Check if frame is compressed */ if (id3_decompress_frame(frame) == -1) return NULL; - - if (frame->fr_desc->fd_id == ID3_WXXX) { + + ID3_FRAME_DEFINE_CURSOR(frame); + + if ( frame->fr_desc->fd_id == ID3_WXXX ) { /* * This is a user defined link frame. Skip the description. */ - switch (*(guint8 *) frame->fr_data) { - case ID3_ENCODING_ISO_8859_1: - { - char *text = (char *) frame->fr_data + 1; - - while (*text != 0) - text++; - - return g_strdup(++text); - } - case ID3_ENCODING_UTF16: - { - gint16 *text16 = (gint16 *) ((glong) frame->fr_data + 1); - - while (*text16 != 0) - text16++; - - return g_strdup((char *) (++text16)); - } - default: + guint8 encoding; + gsize description_size; + + ID3_FRAME_READ_OR_RETVAL(encoding, NULL); + + description_size = id3_string_size(encoding, cursor, length); + if (description_size == 0) return NULL; - } + cursor += description_size; + length -= description_size; } - return g_strdup((char *) frame->fr_data); + return id3_string_decode(ID3_ENCODING_ISO_8859_1, cursor, length); } @@ -102,8 +92,10 @@ if (id3_decompress_frame(frame) == -1) return NULL; - if (*(guint8 *) frame->fr_data == ID3_ENCODING_ISO_8859_1) - return g_strdup((char *) frame->fr_data + 1); - else - return id3_utf16_to_ascii((gint16 *) ((glong) frame->fr_data + 1)); + ID3_FRAME_DEFINE_CURSOR(frame); + + guint8 encoding; + ID3_FRAME_READ_OR_RETVAL(encoding, NULL); + + return id3_string_decode(encoding, cursor, length); } diff -r -U3 bmp-0.9.7.orig/Input/mpg123/xmms-id3.h bmp-0.9.7/Input/mpg123/xmms-id3.h --- bmp-0.9.7.orig/Input/mpg123/xmms-id3.h 2005-02-25 21:45:03.000000000 -0500 +++ bmp-0.9.7/Input/mpg123/xmms-id3.h 2005-02-25 21:49:22.000000000 -0500 @@ -29,6 +29,7 @@ #define ID3_H #include +#include #include /* @@ -318,6 +319,21 @@ #define ID3_WPB ID3_FRAME_ID_22('W', 'P', 'B') #define ID3_WXX ID3_FRAME_ID_22('W', 'X', 'X') +/* + * Handy macros which help us writing more secure length-aware code + * which involves reading the frame's data buffer. + */ + +#define ID3_FRAME_DEFINE_CURSOR(frame) \ + gsize length = frame->fr_size; \ + guint8* cursor = frame->fr_data; + +#define ID3_FRAME_READ_OR_RETVAL(variable, retval) \ + if (length < sizeof(variable)) \ + return retval; \ + memcpy((void*)&variable, (void*)cursor, sizeof(variable)); \ + cursor += sizeof(variable); \ + length -= sizeof(variable); /* * Prototypes. @@ -343,7 +359,8 @@ void id3_frame_clear_data(struct id3_frame *frame); /* From id3_frame_text.c */ -char *id3_utf16_to_ascii(void *); +gsize id3_string_size(guint8 encoding, const void* text, gsize max_size); +gchar* id3_string_decode(guint8 encoding, const void* text, gsize max_size); gint8 id3_get_encoding(struct id3_frame *); int id3_set_encoding(struct id3_frame *, gint8); char *id3_get_text(struct id3_frame *);