mirror of
https://github.com/opencv/opencv.git
synced 2026-07-21 19:33:03 +04:00
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<size_t>(m_width) * static_cast<size_t>(m_bpp); const size_t src_pitch_size = ((bits_per_row + 7) / 8 + 3) & ~static_cast<size_t>(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<size_t>(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
This commit is contained in:
@@ -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<size_t>(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();
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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<size_t>(width) * static_cast<size_t>(bpp);
|
||||
const size_t bytes_per_row = (bits_per_row + 7) / 8;
|
||||
const size_t aligned_pitch = (bytes_per_row + alignment - 1) & ~static_cast<size_t>(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<size_t>(width) * static_cast<size_t>(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)
|
||||
|
||||
@@ -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<size_t>(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
|
||||
|
||||
Reference in New Issue
Block a user