From cbe7384016083eac16078b359acd7a842253d503 Mon Sep 17 00:00:00 2001 From: Young Xiao Date: Sat, 16 Mar 2019 19:57:27 +0800 Subject: [PATCH 1/3] convertbmp: detect invalid file dimensions early width/length dimensions read from bmp headers are not necessarily valid. For instance they may have been maliciously set to very large values with the intention to cause DoS (large memory allocation, stack overflow). In these cases we want to detect the invalid size as early as possible. This commit introduces a counter which verifies that the number of written bytes corresponds to the advertized width/length. See commit 8ee335227bbc for details. Signed-off-by: Young Xiao --- src/bin/jp2/convertbmp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/bin/jp2/convertbmp.c b/src/bin/jp2/convertbmp.c index 0af52f816..ec34f535b 100644 --- a/src/bin/jp2/convertbmp.c +++ b/src/bin/jp2/convertbmp.c @@ -622,13 +622,13 @@ static OPJ_BOOL bmp_read_rle8_data(FILE* IN, OPJ_UINT8* pData, static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, OPJ_UINT32 stride, OPJ_UINT32 width, OPJ_UINT32 height) { - OPJ_UINT32 x, y; + OPJ_UINT32 x, y, written; OPJ_UINT8 *pix; const OPJ_UINT8 *beyond; beyond = pData + stride * height; pix = pData; - x = y = 0U; + x = y = written = 0U; while (y < height) { int c = getc(IN); if (c == EOF) { @@ -642,6 +642,7 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, for (j = 0; (j < c) && (x < width) && ((OPJ_SIZE_T)pix < (OPJ_SIZE_T)beyond); j++, x++, pix++) { *pix = (OPJ_UINT8)((j & 1) ? (c1 & 0x0fU) : ((c1 >> 4) & 0x0fU)); + written++; } } else { /* absolute mode */ c = getc(IN); @@ -671,6 +672,7 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, c1 = (OPJ_UINT8)getc(IN); } *pix = (OPJ_UINT8)((j & 1) ? (c1 & 0x0fU) : ((c1 >> 4) & 0x0fU)); + written++; } if (((c & 3) == 1) || ((c & 3) == 2)) { /* skip padding byte */ getc(IN); @@ -678,6 +680,10 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, } } } /* while(y < height) */ + if (written != width * height) { + fprintf(stderr, "warning, image's actual size does not match advertized one\n"); + return OPJ_FALSE; + } return OPJ_TRUE; } From d93252480d355809288fd37cc60d3599a69fe37c Mon Sep 17 00:00:00 2001 From: Young Xiao Date: Sat, 16 Mar 2019 20:09:59 +0800 Subject: [PATCH 2/3] bmp_read_rle4_data(): avoid potential infinite loop --- src/bin/jp2/convertbmp.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/bin/jp2/convertbmp.c b/src/bin/jp2/convertbmp.c index ec34f535b..2fc4e9bc4 100644 --- a/src/bin/jp2/convertbmp.c +++ b/src/bin/jp2/convertbmp.c @@ -632,12 +632,18 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, while (y < height) { int c = getc(IN); if (c == EOF) { - break; + return OPJ_FALSE; } if (c) { /* encoded mode */ - int j; - OPJ_UINT8 c1 = (OPJ_UINT8)getc(IN); + int j, c1_int; + OPJ_UINT8 c1; + + c1_int = getc(IN); + if (c1_int == EOF) { + return OPJ_FALSE; + } + c1 = (OPJ_UINT8)c1_int; for (j = 0; (j < c) && (x < width) && ((OPJ_SIZE_T)pix < (OPJ_SIZE_T)beyond); j++, x++, pix++) { @@ -647,7 +653,7 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, } else { /* absolute mode */ c = getc(IN); if (c == EOF) { - break; + return OPJ_FALSE; } if (c == 0x00) { /* EOL */ @@ -658,8 +664,14 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, break; } else if (c == 0x02) { /* MOVE by dxdy */ c = getc(IN); + if (c == EOF) { + return OPJ_FALSE; + } x += (OPJ_UINT32)c; c = getc(IN); + if (c == EOF) { + return OPJ_FALSE; + } y += (OPJ_UINT32)c; pix = pData + y * stride + x; } else { /* 03 .. 255 : absolute mode */ @@ -669,13 +681,21 @@ static OPJ_BOOL bmp_read_rle4_data(FILE* IN, OPJ_UINT8* pData, for (j = 0; (j < c) && (x < width) && ((OPJ_SIZE_T)pix < (OPJ_SIZE_T)beyond); j++, x++, pix++) { if ((j & 1) == 0) { - c1 = (OPJ_UINT8)getc(IN); + int c1_int; + c1_int = getc(IN); + if (c1_int == EOF) { + return OPJ_FALSE; + } + c1 = (OPJ_UINT8)c1_int; } *pix = (OPJ_UINT8)((j & 1) ? (c1 & 0x0fU) : ((c1 >> 4) & 0x0fU)); written++; } if (((c & 3) == 1) || ((c & 3) == 2)) { /* skip padding byte */ - getc(IN); + c = getc(IN); + if (c == EOF) { + return OPJ_FALSE; + } } } } From e5640c79c480b9f9853aa775e1b581b58a22479f Mon Sep 17 00:00:00 2001 From: Young Xiao Date: Sat, 16 Mar 2019 20:16:57 +0800 Subject: [PATCH 3/3] solved possible null pointer deference in openjpip --- src/lib/openjpip/channel_manager.c | 3 +++ src/lib/openjpip/target_manager.c | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/lib/openjpip/channel_manager.c b/src/lib/openjpip/channel_manager.c index 69a56f23d..4a9bd94e0 100644 --- a/src/lib/openjpip/channel_manager.c +++ b/src/lib/openjpip/channel_manager.c @@ -175,6 +175,9 @@ channel_param_t * search_channel(const char cid[], { channel_param_t *foundchannel; + if (!cid) + return NULL; + foundchannel = channellist->first; while (foundchannel != NULL) { diff --git a/src/lib/openjpip/target_manager.c b/src/lib/openjpip/target_manager.c index 244d05fab..f1be2de42 100644 --- a/src/lib/openjpip/target_manager.c +++ b/src/lib/openjpip/target_manager.c @@ -226,6 +226,9 @@ target_param_t * search_target(const char targetname[], { target_param_t *foundtarget; + if (!targetname) + return NULL; + foundtarget = targetlist->first; while (foundtarget != NULL) { @@ -244,6 +247,9 @@ target_param_t * search_targetBytid(const char tid[], { target_param_t *foundtarget; + if (!tid) + return NULL; + foundtarget = targetlist->first; while (foundtarget != NULL) {