From c7729066c417b35beac16fbede7401c044625a3d Mon Sep 17 00:00:00 2001 From: Murat Raimbekov Date: Fri, 6 Feb 2026 16:39:55 +0600 Subject: [PATCH] Merge pull request #28379 from raimbekovm:fix-bmp-sunras-overflow imgcodecs: fix integer overflow in BMP and SunRaster decoders #28379 ### Pull Request Readiness Checklist - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake Fixes #28350 ### Description This PR fixes integer overflow vulnerabilities in BMP and SunRaster image decoders that could cause heap buffer over-read when decoding malformed images. ### Root Cause The `src_pitch` and `width3` calculations in `BmpDecoder::readData()` and `SunRasterDecoder::readData()` used `int` arithmetic: ```cpp int src_pitch = ((m_width * m_bpp + 7) / 8 + 3) & -4; int width3 = m_width * nch; ``` When `m_width` or `m_bpp` are large values from a crafted malicious file, the multiplication overflows, resulting in a small or negative value. This leads to insufficient buffer allocation, causing heap buffer over-read during subsequent decoding operations. ### Fix 1. **Use `size_t` arithmetic**: Cast operands to `size_t` before multiplication: ```cpp const size_t bits_per_row = static_cast(m_width) * static_cast(m_bpp); const size_t src_pitch_size = ((bits_per_row + 7) / 8 + 3) & ~static_cast(3); ``` 2. **Add size validation**: Added `CV_CheckLT` to reject images requiring buffers larger than 256MB: ```cpp const size_t MAX_SRC_PITCH = static_cast(1) << 28; CV_CheckLT(src_pitch_size, MAX_SRC_PITCH, "BMP: src_pitch exceeds maximum allowed size"); ``` 3. **Safe conversion**: Use `validateToInt()` for safe conversion back to `int`. ### Files Changed - `modules/imgcodecs/src/grfmt_bmp.cpp` - BmpDecoder::readData() - `modules/imgcodecs/src/grfmt_sunras.cpp` - SunRasterDecoder::readData() ### Testing - Code compiles without warnings - Basic BMP and SunRaster encode/decode tests pass - Overflow conditions are now properly rejected with CV_CheckLT --- modules/imgcodecs/src/grfmt_bmp.cpp | 12 +++++++--- modules/imgcodecs/src/grfmt_sunras.cpp | 11 +++++---- modules/imgcodecs/src/utils.cpp | 31 ++++++++++++++++++++++++++ modules/imgcodecs/src/utils.hpp | 10 +++++++++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/modules/imgcodecs/src/grfmt_bmp.cpp b/modules/imgcodecs/src/grfmt_bmp.cpp index 77201b3af1..c8b604166c 100644 --- a/modules/imgcodecs/src/grfmt_bmp.cpp +++ b/modules/imgcodecs/src/grfmt_bmp.cpp @@ -233,9 +233,13 @@ bool BmpDecoder::readData( Mat& img ) bool color = img.channels() > 1; uchar gray_palette[256] = {0}; bool result = false; - int src_pitch = ((m_width*(m_bpp != 15 ? m_bpp : 16) + 7)/8 + 3) & -4; int nch = color ? 3 : 1; - int y, width3 = m_width*nch; + + const int effective_bpp = (m_bpp != 15) ? m_bpp : 16; + const RowPitchParams pitch_params = calculateRowPitch(m_width, effective_bpp, 4, "BMP"); + const int src_pitch = pitch_params.src_pitch; + const int width3 = calculateRowSize(m_width, nch, "BMP"); + int y; if( m_offset < 0 || !m_strm.isOpened()) return false; @@ -255,7 +259,9 @@ bool BmpDecoder::readData( Mat& img ) { CvtPaletteToGray( m_palette, gray_palette, 1 << m_bpp ); } - _bgr.allocate(m_width*3 + 32); + const size_t bgr_size = static_cast(m_width) * 3 + 32; + CV_CheckLT(bgr_size, MAX_IMAGE_ROW_SIZE, "BMP: bgr buffer size exceeds maximum allowed size"); + _bgr.allocate(bgr_size); } uchar *src = _src.data(), *bgr = _bgr.data(); diff --git a/modules/imgcodecs/src/grfmt_sunras.cpp b/modules/imgcodecs/src/grfmt_sunras.cpp index 0dd31b3b1a..06d55d2c44 100644 --- a/modules/imgcodecs/src/grfmt_sunras.cpp +++ b/modules/imgcodecs/src/grfmt_sunras.cpp @@ -133,10 +133,13 @@ bool SunRasterDecoder::readData( Mat& img ) size_t step = img.step; uchar gray_palette[256] = {0}; bool result = false; - int src_pitch = ((m_width*m_bpp + 7)/8 + 1) & -2; int nch = color ? 3 : 1; - int width3 = m_width*nch; - int y; + + const RowPitchParams pitch_params = calculateRowPitch(m_width, m_bpp, 2, "SunRaster"); + const int src_pitch = pitch_params.src_pitch; + const size_t bytes_per_row = pitch_params.bytes_per_row; + const int width3 = calculateRowSize(m_width, nch, "SunRaster"); + int y; if( m_offset < 0 || !m_strm.isOpened()) return false; @@ -169,7 +172,7 @@ bool SunRasterDecoder::readData( Mat& img ) } else { - uchar* line_end = src + (m_width*m_bpp + 7)/8; + uchar* line_end = src + bytes_per_row; uchar* tsrc = src; y = 0; diff --git a/modules/imgcodecs/src/utils.cpp b/modules/imgcodecs/src/utils.cpp index 3b597fe61c..5c3127792a 100644 --- a/modules/imgcodecs/src/utils.cpp +++ b/modules/imgcodecs/src/utils.cpp @@ -51,6 +51,37 @@ int validateToInt(size_t sz) return valueInt; } +RowPitchParams calculateRowPitch(int width, int bpp, int alignment, const char* format_name) +{ + CV_Assert(width > 0 && bpp > 0 && alignment > 0); + CV_Assert((alignment & (alignment - 1)) == 0); // must be power of 2 + + const size_t bits_per_row = static_cast(width) * static_cast(bpp); + const size_t bytes_per_row = (bits_per_row + 7) / 8; + const size_t aligned_pitch = (bytes_per_row + alignment - 1) & ~static_cast(alignment - 1); + + if (aligned_pitch >= MAX_IMAGE_ROW_SIZE) + CV_Error(cv::Error::StsOutOfRange, + cv::format("%s: src_pitch exceeds maximum allowed size", format_name)); + + RowPitchParams result; + result.src_pitch = validateToInt(aligned_pitch); + result.bytes_per_row = bytes_per_row; + return result; +} + +int calculateRowSize(int width, int nch, const char* format_name) +{ + CV_Assert(width > 0 && nch > 0); + + const size_t row_size = static_cast(width) * static_cast(nch); + if (row_size >= MAX_IMAGE_ROW_SIZE) + CV_Error(cv::Error::StsOutOfRange, + cv::format("%s: row size exceeds maximum allowed size", format_name)); + + return validateToInt(row_size); +} + #define SCALE 14 #define cR (int)(0.299*(1 << SCALE) + 0.5) #define cG (int)(0.587*(1 << SCALE) + 0.5) diff --git a/modules/imgcodecs/src/utils.hpp b/modules/imgcodecs/src/utils.hpp index 8b0ad1a9e9..3ed18eaaba 100644 --- a/modules/imgcodecs/src/utils.hpp +++ b/modules/imgcodecs/src/utils.hpp @@ -135,6 +135,16 @@ uchar* FillGrayRow4( uchar* data, uchar* indices, int len, uchar* palette ); uchar* FillColorRow1( uchar* data, uchar* indices, int len, PaletteEntry* palette ); uchar* FillGrayRow1( uchar* data, uchar* indices, int len, uchar* palette ); +static const size_t MAX_IMAGE_ROW_SIZE = static_cast(1) << 28; // 256 MB + +struct RowPitchParams { + int src_pitch; + size_t bytes_per_row; +}; + +RowPitchParams calculateRowPitch(int width, int bpp, int alignment, const char* format_name); +int calculateRowSize(int width, int nch, const char* format_name); + CV_INLINE bool isBigEndian( void ) { #ifdef WORDS_BIGENDIAN