From e44890e04830a8a2e37d8ead43b12f13900d2030 Mon Sep 17 00:00:00 2001 From: Minje Ahn Date: Tue, 18 Jul 2017 09:59:53 +0900 Subject: [PATCH] [SATIZENVUL-914] Add verify for value range Change-Id: I6ffdc7c29568471347d4f58841925096a31a66e2 Signed-off-by: Minje Ahn --- formats/ffmpeg/mm_file_format_ffmpeg_mem.c | 2 +- formats/ffmpeg/mm_file_formats.c | 5 +-- include/mm_file_debug.h | 3 ++ tests/mm_file_traverse.h | 3 ++ tests/mm_file_traverser.c | 8 ++-- utils/mm_file_util_io_mem.c | 2 +- utils/mm_file_util_io_mmap.c | 2 +- utils/mm_file_util_tag.c | 60 ++++++++++++++---------------- 8 files changed, 42 insertions(+), 43 deletions(-) diff --git a/formats/ffmpeg/mm_file_format_ffmpeg_mem.c b/formats/ffmpeg/mm_file_format_ffmpeg_mem.c index 99ddcfe..c19b095 100755 --- a/formats/ffmpeg/mm_file_format_ffmpeg_mem.c +++ b/formats/ffmpeg/mm_file_format_ffmpeg_mem.c @@ -109,7 +109,7 @@ static int mmf_mem_read(URLContext *h, unsigned char *buf, int size) int len = 0; - if (!h || !h->priv_data || !buf) { + if (!h || !h->priv_data || !buf || size <= 0) { debug_error(DEBUG, "invalid para\n"); return MMFILE_UTIL_FAIL; } diff --git a/formats/ffmpeg/mm_file_formats.c b/formats/ffmpeg/mm_file_formats.c index b2e9c2b..d5998f7 100755 --- a/formats/ffmpeg/mm_file_formats.c +++ b/formats/ffmpeg/mm_file_formats.c @@ -315,9 +315,8 @@ _PreprocessFile(MMFileSourceType *fileSrc, char **urifilename, int *formatEnum) } memset(*urifilename, 0x00, uriLen + filename_len + 1); - strncpy(*urifilename, uriStr, uriLen); - strncat(*urifilename, fileName, filename_len); - (*urifilename)[uriLen + filename_len] = '\0'; + SAFE_STRLCPY(*urifilename, uriStr, uriLen + filename_len + 1); + SAFE_STRLCAT(*urifilename, fileName, uriLen + filename_len + 1); /** * Get file extension from file's name diff --git a/include/mm_file_debug.h b/include/mm_file_debug.h index 2906c41..10c58d8 100755 --- a/include/mm_file_debug.h +++ b/include/mm_file_debug.h @@ -42,6 +42,9 @@ extern "C" { #define DEBUG_MODE 0 #endif +#define SAFE_STRLCPY(dst, src, n) g_strlcpy(dst, src, n); +#define SAFE_STRLCAT(dst, src, n) g_strlcat(dst, src, n); + /*#define LOG_COLOR */ #ifdef LOG_COLOR diff --git a/tests/mm_file_traverse.h b/tests/mm_file_traverse.h index 6d051b1..0a770a8 100755 --- a/tests/mm_file_traverse.h +++ b/tests/mm_file_traverse.h @@ -29,6 +29,9 @@ typedef enum { MMFILE_SUCCESS } MMFILE_RETURN; +#define SAFE_STRLCPY(dst, src, n) g_strlcpy(dst, src, n); +#define SAFE_STRLCAT(dst, src, n) g_strlcat(dst, src, n); + typedef int (*MMFunc)(void *data, void *user_data, bool file_test); int mmfile_get_file_names(char *root_dir, MMFunc cbfunc, void *user_data); diff --git a/tests/mm_file_traverser.c b/tests/mm_file_traverser.c index 1006120..2c69143 100755 --- a/tests/mm_file_traverser.c +++ b/tests/mm_file_traverser.c @@ -64,7 +64,7 @@ int mmfile_get_file_names(char *root_dir, MMFunc cbfunc, void *user_data) gpointer element_data = NULL; while ((element_data = g_list_nth_data(g_directories, i)) != NULL) { if (strlen((char *) element_data) > 0 && strlen((char *) element_data) <= MMFILE_PATH_MAX) { - strncpy(pdirname, (char *) element_data, strlen((char *) element_data)); + SAFE_STRLCPY(pdirname, (char *) element_data, sizeof(pdirname)); if ((dp = opendir(pdirname)) != NULL) { while ((dirp = readdir(dp)) != NULL) { @@ -76,9 +76,9 @@ int mmfile_get_file_names(char *root_dir, MMFunc cbfunc, void *user_data) } memset(cdirname, 0x00, MMFILE_PATH_MAX + 1); - strncpy(cdirname, pdirname, strlen(pdirname)); - strncat(cdirname, "/", 1); - strncat(cdirname, dirp->d_name, strlen(dirp->d_name)); + SAFE_STRLCPY(cdirname, pdirname, sizeof(cdirname)); + SAFE_STRLCAT(cdirname, "/", sizeof(cdirname)); + SAFE_STRLCAT(cdirname, dirp->d_name, sizeof(cdirname)); if (lstat(cdirname, &statbuf) < 0) { printf("lstat error\n"); diff --git a/utils/mm_file_util_io_mem.c b/utils/mm_file_util_io_mem.c index e0034f0..b134080 100755 --- a/utils/mm_file_util_io_mem.c +++ b/utils/mm_file_util_io_mem.c @@ -98,7 +98,7 @@ static int mmf_mem_read(MMFileIOHandle *h, unsigned char *buf, int size) const unsigned char *c = NULL; int len = 0; - if (!h || !h->privateData || !buf) { + if (!h || !h->privateData || !buf || size <= 0) { debug_error(DEBUG, "invalid para\n"); return MMFILE_IO_FAILED; } diff --git a/utils/mm_file_util_io_mmap.c b/utils/mm_file_util_io_mmap.c index 886d9c4..bff44d6 100755 --- a/utils/mm_file_util_io_mmap.c +++ b/utils/mm_file_util_io_mmap.c @@ -139,7 +139,7 @@ static int mmf_mmap_read(MMFileIOHandle *h, unsigned char *buf, int size) const unsigned char *c = NULL; int len = 0; - if (!h || !h->privateData || !buf) { + if (!h || !h->privateData || !buf || size <= 0) { debug_error(DEBUG, "invalid para\n"); return MMFILE_IO_FAILED; } diff --git a/utils/mm_file_util_tag.c b/utils/mm_file_util_tag.c index 4766c6a..fe5f672 100755 --- a/utils/mm_file_util_tag.c +++ b/utils/mm_file_util_tag.c @@ -2311,8 +2311,7 @@ bool mm_file_id3tag_parse_v222(AvFileContentInfo *pInfo, unsigned char *buffer) 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; + SAFE_STRLCPY(pInfo->pGenre, tmp_genre, tmp_genre_len + 1); } } } @@ -2510,7 +2509,7 @@ bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer) unsigned long purelyFramelen = 0; unsigned int encodingOffSet = 0; int inx = 0, realCpyFrameNum = 0, checkImgMimeTypeMax = 0, imgstartOffset = 0, tmp = 0; - int textEncodingType = 0; + unsigned int textEncodingType = 0; char **charset_array = NULL; const char *MIME_PRFIX = "image/"; @@ -2917,8 +2916,7 @@ bool mm_file_id3tag_parse_v223(AvFileContentInfo *pInfo, unsigned char *buffer) 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; + SAFE_STRLCPY(pInfo->pGenre, tmp_genre, tmp_genre_len + 1); } } } @@ -3129,7 +3127,7 @@ bool mm_file_id3tag_parse_v224(AvFileContentInfo *pInfo, unsigned char *buffer) unsigned long purelyFramelen = 0; unsigned int encodingOffSet = 0; int inx = 0, realCpyFrameNum = 0, checkImgMimeTypeMax = 0, imgstartOffset = 0, tmp = 0; - int textEncodingType = 0; + unsigned int textEncodingType = 0; char **charset_array = NULL; const char *MIME_PRFIX = "image/"; @@ -3450,25 +3448,26 @@ bool mm_file_id3tag_parse_v224(AvFileContentInfo *pInfo, unsigned char *buffer) } else { for (idx = 0; idx < realCpyFrameNum; idx++) { if (pExtContent[tmp + idx] == 0x00) { - synclyrics_info = (AvSynclyricsInfo *)malloc(sizeof(AvSynclyricsInfo)); - - if (textEncodingType == AV_ID3V2_UTF8) { - synclyrics_info->lyric_info = mmfile_malloc(copy_len + 1); - if (synclyrics_info->lyric_info) { - memset(synclyrics_info->lyric_info, 0, copy_len + 1); - memcpy(synclyrics_info->lyric_info, pExtContent + copy_start_pos, copy_len); - synclyrics_info->lyric_info[copy_len] = '\0'; + synclyrics_info = (AvSynclyricsInfo *)mmfile_malloc(sizeof(AvSynclyricsInfo)); + if (synclyrics_info) { + if (textEncodingType == AV_ID3V2_UTF8) { + synclyrics_info->lyric_info = mmfile_malloc(copy_len + 1); + if (synclyrics_info->lyric_info) { + memset(synclyrics_info->lyric_info, 0, copy_len + 1); + memcpy(synclyrics_info->lyric_info, pExtContent + copy_start_pos, copy_len); + synclyrics_info->lyric_info[copy_len] = '\0'; + } + } else { + synclyrics_info->lyric_info = mmfile_string_convert((const char *)&pExtContent[copy_start_pos], copy_len, "UTF-8", charset_array[textEncodingType], NULL, NULL); } - } else { - synclyrics_info->lyric_info = mmfile_string_convert((const char *)&pExtContent[copy_start_pos], copy_len, "UTF-8", charset_array[textEncodingType], NULL, NULL); - } - synclyrics_info->time_info = (unsigned long)pExtContent[tmp + idx + 1] << 24 | (unsigned long)pExtContent[tmp + idx + 2] << 16 | (unsigned long)pExtContent[tmp + idx + 3] << 8 | (unsigned long)pExtContent[tmp + idx + 4]; - idx += 4; - copy_start_pos = tmp + idx + 1; - debug_msg(RELEASE, "[%lu][%s] idx[%d], copy_len[%d] copy_start_pos[%d]", synclyrics_info->time_info, synclyrics_info->lyric_info, idx, copy_len, copy_start_pos); - copy_len = 0; - synclyrics_info_list = g_list_append(synclyrics_info_list, synclyrics_info); + synclyrics_info->time_info = (unsigned long)pExtContent[tmp + idx + 1] << 24 | (unsigned long)pExtContent[tmp + idx + 2] << 16 | (unsigned long)pExtContent[tmp + idx + 3] << 8 | (unsigned long)pExtContent[tmp + idx + 4]; + idx += 4; + copy_start_pos = tmp + idx + 1; + debug_msg(RELEASE, "[%lu][%s] idx[%d], copy_len[%d] copy_start_pos[%d]", synclyrics_info->time_info, synclyrics_info->lyric_info, idx, copy_len, copy_start_pos); + copy_len = 0; + synclyrics_info_list = g_list_append(synclyrics_info_list, synclyrics_info); + } } copy_len++; } @@ -3581,8 +3580,7 @@ bool mm_file_id3tag_parse_v224(AvFileContentInfo *pInfo, unsigned char *buffer) 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; + SAFE_STRLCPY(pInfo->pGenre, tmp_genre, tmp_genre_len + 1); } } } @@ -3887,8 +3885,7 @@ void mm_file_id3tag_restore_content_info(AvFileContentInfo *pInfo) /* Give space for NULL character. Hence added "+1" */ pInfo->pGenre = mmfile_malloc(sizeof(char) * (pInfo->genreLen + 1)); if (pInfo->pGenre) { - strncpy(pInfo->pGenre, MpegAudio_Genre[genre], pInfo->genreLen); - pInfo->pGenre[pInfo->genreLen] = '\0'; + SAFE_STRLCPY(pInfo->pGenre, MpegAudio_Genre[genre], pInfo->genreLen + 1); } } } @@ -3906,8 +3903,7 @@ void mm_file_id3tag_restore_content_info(AvFileContentInfo *pInfo) pInfo->genreLen = strlen(pInfo->pGenre); mpegAudioGenre = mmfile_malloc(sizeof(char) * (pInfo->genreLen + 1)); if (mpegAudioGenre != NULL) { - mpegAudioGenre[pInfo->genreLen] = '\0'; - strncpy(mpegAudioGenre, pInfo->pGenre, pInfo->genreLen); + SAFE_STRLCPY(mpegAudioGenre, pInfo->pGenre, pInfo->genreLen + 1); } } else { pInfo->genreLen = 0; @@ -3946,8 +3942,7 @@ void mm_file_id3tag_restore_content_info(AvFileContentInfo *pInfo) /* Give space for NULL character. Hence added "+1" */ pInfo->pGenre = mmfile_malloc(sizeof(char) * (pInfo->genreLen + 1)); if (pInfo->pGenre) { - strncpy(pInfo->pGenre, MpegAudio_Genre[idv2IntGenre], pInfo->genreLen); - pInfo->pGenre[pInfo->genreLen] = 0; + SAFE_STRLCPY(pInfo->pGenre, MpegAudio_Genre[idv2IntGenre], pInfo->genreLen + 1); } } } @@ -3960,8 +3955,7 @@ void mm_file_id3tag_restore_content_info(AvFileContentInfo *pInfo) /* Give space for NULL character. Hence added "+1" */ pInfo->pGenre = mmfile_malloc(sizeof(char) * (pInfo->genreLen + 1)); if (pInfo->pGenre) { - strncpy(pInfo->pGenre, mpegAudioGenre, pInfo->genreLen); - pInfo->pGenre[pInfo->genreLen] = '\0'; + SAFE_STRLCPY(pInfo->pGenre, mpegAudioGenre, pInfo->genreLen + 1); } debug_msg(RELEASE, "pInfo->pGenre = %s, pInfo->genreLen = %d\n", pInfo->pGenre, pInfo->genreLen); } else { -- 2.7.4