From 058bcdc5d95e54161661e2b870d9cd1cf9b51e19 Mon Sep 17 00:00:00 2001 From: Jiyong Min Date: Thu, 20 Apr 2017 10:22:24 +0900 Subject: [PATCH] Modify to use vulnerable function 'sscanf' Change-Id: I82d34b900905e3dc487a7b22b995c5c05b609ec1 Signed-off-by: Jiyong Min (cherry picked from commit 81d593deaba59483538a1f357b676aa805807304) --- packaging/libmm-fileinfo.spec | 2 +- utils/mm_file_util_tag.c | 155 +++++++++++++++++++++------------- 2 files changed, 99 insertions(+), 58 deletions(-) diff --git a/packaging/libmm-fileinfo.spec b/packaging/libmm-fileinfo.spec index fa40aa4..7baba6b 100755 --- a/packaging/libmm-fileinfo.spec +++ b/packaging/libmm-fileinfo.spec @@ -1,6 +1,6 @@ Name: libmm-fileinfo Summary: Media Fileinfo -Version: 0.6.59 +Version: 0.6.60 Release: 0 Group: System/Libraries License: Apache-2.0 diff --git a/utils/mm_file_util_tag.c b/utils/mm_file_util_tag.c index 8558d0f..e486614 100755 --- a/utils/mm_file_util_tag.c +++ b/utils/mm_file_util_tag.c @@ -1538,6 +1538,35 @@ char *rtrimN(char *pStr) return strdup(pStr); } +bool safe_atoi(char *buffer, int *si) + +{ + char *end; + errno = 0; + + const long sl = strtol(buffer, &end, 10); + + if (end == buffer) { + debug_error(RELEASE, "not a decimal number"); + return FALSE; + } else if ('\0' != *end) { + debug_error(RELEASE, "extra characters at end of input: %s", end); + return FALSE; + } else if ((LONG_MIN == sl || LONG_MAX == sl) && (ERANGE == errno)) { + debug_error(RELEASE, "out of range of type long"); + return FALSE; + } else if (sl > INT_MAX) { + debug_error(RELEASE, "greater than INT_MAX"); + return FALSE; + } else if (sl < INT_MIN) { + debug_error(RELEASE, "less than INT_MIN"); + return FALSE; + } else { + *si = (int)sl; + } + return TRUE; +} + static bool make_characterset_array(char ***charset_array) { char *locale = MMFileUtilGetLocale(NULL); @@ -1829,27 +1858,31 @@ bool mm_file_id3tag_parse_v222(AvFileContentInfo *pInfo, unsigned char *buffer) ret = is_numeric(pInfo->pGenre, pInfo->genreLen); if (ret == TRUE) { - sscanf(pInfo->pGenre, "%d", &int_genre); - debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); - - /*Change int to string */ - if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { - /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ - char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ - int tmp_genre_len = 0; - - memset(tmp_genre, 0, 6); - snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); - - tmp_genre_len = strlen(tmp_genre); - if (tmp_genre_len > 0) { - if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); - pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); - if (pInfo->pGenre) { - strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); - pInfo->pGenre[tmp_genre_len] = 0; + ret = safe_atoi(pInfo->pGenre, &int_genre); + if (ret == TRUE) { + debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); + + /*Change int to string */ + if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { + /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ + char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ + int tmp_genre_len = 0; + + memset(tmp_genre, 0, 6); + snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); + + tmp_genre_len = strlen(tmp_genre); + if (tmp_genre_len > 0) { + if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); + pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); + if (pInfo->pGenre) { + strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); + pInfo->pGenre[tmp_genre_len] = 0; + } } } + } else { + debug_error(RELEASE, "genre information is wrong inteager [%s]\n", pInfo->pGenre); } } } @@ -2430,27 +2463,31 @@ bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer) ret = is_numeric(pInfo->pGenre, pInfo->genreLen); if (ret == TRUE) { - sscanf(pInfo->pGenre, "%d", &int_genre); - debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); - - /*Change int to string */ - if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { - /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ - char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ - int tmp_genre_len = 0; - - memset(tmp_genre, 0, 6); - snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); - - tmp_genre_len = strlen(tmp_genre); - if (tmp_genre_len > 0) { - if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); - pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); - if (pInfo->pGenre) { - strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); - pInfo->pGenre[tmp_genre_len] = 0; + ret = safe_atoi(pInfo->pGenre, &int_genre); + if (ret == TRUE) { + debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); + + /*Change int to string */ + if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { + /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ + char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ + int tmp_genre_len = 0; + + memset(tmp_genre, 0, 6); + snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); + + tmp_genre_len = strlen(tmp_genre); + if (tmp_genre_len > 0) { + if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); + pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); + if (pInfo->pGenre) { + strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); + pInfo->pGenre[tmp_genre_len] = 0; + } } } + } else { + debug_msg(RELEASE, "genre information is wrong inteager [%s]\n", pInfo->pGenre); } } } @@ -3087,27 +3124,31 @@ bool mm_file_id3tag_parse_v224(AvFileContentInfo *pInfo, unsigned char *buffer) ret = is_numeric(pInfo->pGenre, pInfo->genreLen); if (ret == TRUE) { - sscanf(pInfo->pGenre, "%d", &int_genre); - debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); - - /*Change int to string */ - if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { - /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ - char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ - int tmp_genre_len = 0; - - memset(tmp_genre, 0, 6); - snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); - - tmp_genre_len = strlen(tmp_genre); - if (tmp_genre_len > 0) { - if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); - pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); - if (pInfo->pGenre) { - strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); - pInfo->pGenre[tmp_genre_len] = 0; + ret = safe_atoi(pInfo->pGenre, &int_genre); + if (ret == TRUE) { + debug_msg(RELEASE, "genre information is inteager [%d]\n", int_genre); + + /*Change int to string */ + if ((0 <= int_genre) && (int_genre < GENRE_COUNT - 1)) { + /*save genreinfo like "(123)". mm_file_id3tag_restore_content_info convert it to string*/ + char tmp_genre[6] = {0, }; /*ex. "(123)+NULL"*/ + int tmp_genre_len = 0; + + memset(tmp_genre, 0, 6); + snprintf(tmp_genre, sizeof(tmp_genre), "(%d)", int_genre); + + tmp_genre_len = strlen(tmp_genre); + if (tmp_genre_len > 0) { + if (pInfo->pGenre) _FREE_EX(pInfo->pGenre); + pInfo->pGenre = mmfile_malloc(sizeof(char) * (tmp_genre_len + 1)); + if (pInfo->pGenre) { + strncpy(pInfo->pGenre, tmp_genre, tmp_genre_len); + pInfo->pGenre[tmp_genre_len] = 0; + } } } + } else { + debug_msg(RELEASE, "genre information is wrong inteager [%s]\n", pInfo->pGenre); } } } -- 2.34.1