From 443059e371e379799a8ac72d8e87464d909b3464 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Thu, 21 Dec 2017 01:10:24 +0000 Subject: [PATCH 1/3] imgcodecs(pxm): fix memcpy size 7bbe1a53cfc097b82b1589f7915a2120de39274c --- modules/highgui/src/grfmt_pxm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/highgui/src/grfmt_pxm.cpp b/modules/highgui/src/grfmt_pxm.cpp index e609d1659662..36f9a059e573 100644 --- a/modules/highgui/src/grfmt_pxm.cpp +++ b/modules/highgui/src/grfmt_pxm.cpp @@ -331,7 +331,7 @@ bool PxMDecoder::readData( Mat& img ) } } else - memcpy( data, src, m_width*(bit_depth/8) ); + memcpy(data, src, img.elemSize1()*m_width); } else { From cd64b504b8f568a923adc764b1fc6303dcd75bec Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Tue, 9 Jan 2018 17:56:52 +0300 Subject: [PATCH 2/3] imgcodecs: add overflow checks imgcodecs: remove assert() usage Origin commits: - be5247921da02e58aa42830c81730ef20a23af80 - 8a76fadaa39b87d740ec3346d9eccb64bde5a6af --- modules/highgui/src/bitstrm.cpp | 22 ++++++++++++++-------- modules/highgui/src/grfmt_bmp.cpp | 1 + modules/highgui/src/grfmt_sunras.cpp | 8 ++++---- modules/highgui/src/precomp.hpp | 2 +- modules/highgui/src/utils.cpp | 2 +- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/modules/highgui/src/bitstrm.cpp b/modules/highgui/src/bitstrm.cpp index 67792a27f287..89f52d4a1bf6 100644 --- a/modules/highgui/src/bitstrm.cpp +++ b/modules/highgui/src/bitstrm.cpp @@ -42,6 +42,7 @@ #include "precomp.hpp" #include "bitstrm.hpp" +#include "utils.hpp" namespace cv { @@ -164,7 +165,7 @@ void RBaseStream::release() void RBaseStream::setPos( int pos ) { - assert( isOpened() && pos >= 0 ); + CV_Assert(isOpened() && pos >= 0); if( !m_file ) { @@ -181,14 +182,19 @@ void RBaseStream::setPos( int pos ) int RBaseStream::getPos() { - assert( isOpened() ); - return m_block_pos + (int)(m_current - m_start); + CV_Assert(isOpened()); + int pos = validateToInt((m_current - m_start) + m_block_pos); + CV_Assert(pos >= m_block_pos); // overflow check + CV_Assert(pos >= 0); // overflow check + return pos; } void RBaseStream::skip( int bytes ) { - assert( bytes >= 0 ); + CV_Assert(bytes >= 0); + uchar* old = m_current; m_current += bytes; + CV_Assert(m_current >= old); // overflow check } ///////////////////////// RLByteStream //////////////////////////// @@ -220,7 +226,7 @@ int RLByteStream::getBytes( void* buffer, int count ) { uchar* data = (uchar*)buffer; int readed = 0; - assert( count >= 0 ); + CV_Assert(count >= 0); while( count > 0 ) { @@ -371,7 +377,7 @@ void WBaseStream::writeBlock() { int size = (int)(m_current - m_start); - assert( isOpened() ); + CV_Assert(isOpened()); if( size == 0 ) return; @@ -442,7 +448,7 @@ void WBaseStream::release() int WBaseStream::getPos() { - assert( isOpened() ); + CV_Assert(isOpened()); return m_block_pos + (int)(m_current - m_start); } @@ -465,7 +471,7 @@ void WLByteStream::putBytes( const void* buffer, int count ) { uchar* data = (uchar*)buffer; - assert( data && m_current && count >= 0 ); + CV_Assert(data && m_current && count >= 0); while( count ) { diff --git a/modules/highgui/src/grfmt_bmp.cpp b/modules/highgui/src/grfmt_bmp.cpp index d17667eff6d2..8897898674f6 100644 --- a/modules/highgui/src/grfmt_bmp.cpp +++ b/modules/highgui/src/grfmt_bmp.cpp @@ -92,6 +92,7 @@ bool BmpDecoder::readHeader() m_offset = m_strm.getDWord(); int size = m_strm.getDWord(); + CV_Assert(size > 0); // overflow, 2Gb limit if( size >= 36 ) { diff --git a/modules/highgui/src/grfmt_sunras.cpp b/modules/highgui/src/grfmt_sunras.cpp index 60a93a0ff8d2..dd854002add1 100644 --- a/modules/highgui/src/grfmt_sunras.cpp +++ b/modules/highgui/src/grfmt_sunras.cpp @@ -120,7 +120,7 @@ bool SunRasterDecoder::readHeader() m_type = IsColorPalette( m_palette, m_bpp ) ? CV_8UC3 : CV_8UC1; m_offset = m_strm.getPos(); - assert( m_offset == 32 + m_maplength ); + CV_Assert(m_offset == 32 + m_maplength); result = true; } } @@ -133,7 +133,7 @@ bool SunRasterDecoder::readHeader() m_offset = m_strm.getPos(); - assert( m_offset == 32 + m_maplength ); + CV_Assert(m_offset == 32 + m_maplength); result = true; } } @@ -226,7 +226,7 @@ bool SunRasterDecoder::readData( Mat& img ) code = m_strm.getByte(); if( len > line_end - tsrc ) { - assert(0); + CV_Error(CV_StsInternal, ""); goto bad_decoding_1bpp; } @@ -367,7 +367,7 @@ bool SunRasterDecoder::readData( Mat& img ) result = true; break; default: - assert(0); + CV_Error(CV_StsInternal, ""); } } catch( ... ) diff --git a/modules/highgui/src/precomp.hpp b/modules/highgui/src/precomp.hpp index af5383b142a0..d8437120039b 100644 --- a/modules/highgui/src/precomp.hpp +++ b/modules/highgui/src/precomp.hpp @@ -54,7 +54,7 @@ #include #include #include -#include +#include // FIX IT: remove this #if defined WIN32 || defined WINCE #if !defined _WIN32_WINNT diff --git a/modules/highgui/src/utils.cpp b/modules/highgui/src/utils.cpp index 68b6d8ec0607..967891128e07 100644 --- a/modules/highgui/src/utils.cpp +++ b/modules/highgui/src/utils.cpp @@ -670,7 +670,7 @@ cvConvertImage( const CvArr* srcarr, CvArr* dstarr, int flags ) icvCvt_BGR2Gray_8u_C3C1R( s, s_step, d, d_step, size, swap_rb ); break; case 33: - assert( swap_rb ); + CV_Assert(swap_rb); icvCvt_RGB2BGR_8u_C3R( s, s_step, d, d_step, size ); break; case 41: From 56072c4406ad051c414e66460b4e73961ecdd3cf Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Tue, 9 Jan 2018 17:36:57 +0300 Subject: [PATCH 3/3] imgcodecs: add more Jasper checks for supported and tested cases 435a3e337bd9d4e11af61cf8b8afca067bf1a8aa --- modules/highgui/src/grfmt_jpeg2000.cpp | 47 ++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/modules/highgui/src/grfmt_jpeg2000.cpp b/modules/highgui/src/grfmt_jpeg2000.cpp index 3b513f653647..b709e62b4edb 100644 --- a/modules/highgui/src/grfmt_jpeg2000.cpp +++ b/modules/highgui/src/grfmt_jpeg2000.cpp @@ -77,7 +77,8 @@ static JasperInitializer initialize_jasper; Jpeg2KDecoder::Jpeg2KDecoder() { - m_signature = '\0' + string() + '\0' + string() + '\0' + string("\x0cjP \r\n\x87\n"); + static const unsigned char signature_[12] = { 0, 0, 0, 0x0c, 'j', 'P', ' ', ' ', 13, 10, 0x87, 10}; + m_signature = string((const char*)signature_, (const char*)signature_ + sizeof(signature_)); m_stream = 0; m_image = 0; } @@ -121,6 +122,8 @@ bool Jpeg2KDecoder::readHeader() jas_image_t* image = jas_image_decode( stream, -1, 0 ); m_image = image; if( image ) { + CV_Assert(0 == (jas_image_tlx(image)) && "not supported"); + CV_Assert(0 == (jas_image_tly(image)) && "not supported"); m_width = jas_image_width( image ); m_height = jas_image_height( image ); @@ -130,14 +133,31 @@ bool Jpeg2KDecoder::readHeader() for( int i = 0; i < numcmpts; i++ ) { int depth_i = jas_image_cmptprec( image, i ); + CV_Assert(depth == 0 || depth == depth_i); // component data type mismatch depth = MAX(depth, depth_i); if( jas_image_cmpttype( image, i ) > 2 ) continue; + int sgnd = jas_image_cmptsgnd(image, i); + int xstart = jas_image_cmpttlx(image, i); + int xend = jas_image_cmptbrx(image, i); + int xstep = jas_image_cmpthstep(image, i); + int ystart = jas_image_cmpttly(image, i); + int yend = jas_image_cmptbry(image, i); + int ystep = jas_image_cmptvstep(image, i); + CV_Assert(sgnd == 0 && "not supported"); + CV_Assert(xstart == 0 && "not supported"); + CV_Assert(ystart == 0 && "not supported"); + CV_Assert(xstep == 1 && "not supported"); + CV_Assert(ystep == 1 && "not supported"); + CV_Assert(xend == m_width); + CV_Assert(yend == m_height); cntcmpts++; } if( cntcmpts ) { + CV_Assert(depth == 8 || depth == 16); + CV_Assert(cntcmpts == 1 || cntcmpts == 3); m_type = CV_MAKETYPE(depth <= 8 ? CV_8U : CV_16U, cntcmpts > 1 ? 3 : 1); result = true; } @@ -150,9 +170,15 @@ bool Jpeg2KDecoder::readHeader() return result; } +static void Jpeg2KDecoder_close(Jpeg2KDecoder* ptr) +{ + ptr->close(); +} +template<> void Ptr::delete_obj() { Jpeg2KDecoder_close(obj); } bool Jpeg2KDecoder::readData( Mat& img ) { + Ptr close_this(this); // auto cleanup: Jpeg2KDecoder_close bool result = false; int color = img.channels() > 1; uchar* data = img.data; @@ -204,11 +230,16 @@ bool Jpeg2KDecoder::readData( Mat& img ) result = true; } else - fprintf(stderr, "JPEG 2000 LOADER ERROR: cannot convert colorspace\n"); + { + jas_cmprof_destroy(clrprof); + CV_Error(CV_StsError, "JPEG 2000 LOADER ERROR: cannot convert colorspace"); + } jas_cmprof_destroy( clrprof ); } else - fprintf(stderr, "JPEG 2000 LOADER ERROR: unable to create colorspace\n"); + { + CV_Error(CV_StsError, "JPEG 2000 LOADER ERROR: unable to create colorspace"); + } } else result = true; @@ -257,8 +288,8 @@ bool Jpeg2KDecoder::readData( Mat& img ) result = readComponent16u( ((unsigned short *)data) + i, buffer, validateToInt(step / 2), cmptlut[i], maxval, offset, ncmpts ); if( !result ) { - i = ncmpts; - result = false; + jas_matrix_destroy( buffer ); + CV_Error(CV_StsError, "JPEG2000 LOADER ERROR: failed to read component"); } } jas_matrix_destroy( buffer ); @@ -267,10 +298,12 @@ bool Jpeg2KDecoder::readData( Mat& img ) } } else - fprintf(stderr, "JPEG2000 LOADER ERROR: colorspace conversion failed\n" ); + { + CV_Error(CV_StsError, "JPEG2000 LOADER ERROR: colorspace conversion failed"); + } } - close(); + CV_Assert(result == true); #ifndef WIN32 if (!clr.empty())