Merge pull request #26739 from vrabaud:png_leak

Add more boundary checks. #26739

Also fix a bug in read_chunk where we could end up with png_get_uint_32(len) + 12 < 4

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [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
- [ ] 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
This commit is contained in:
Vincent Rabaud 2025-01-10 09:33:43 +01:00 committed by GitHub
parent bdb6a968ce
commit d12fa37eed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 33 additions and 23 deletions

View File

@ -255,11 +255,14 @@ bool PngDecoder::readHeader()
png_init_io(png_ptr, m_f);
}
if (read_from_io(&sig, 8, 1) != 1)
// Read PNG header: 137 80 78 71 13 10 26 10
if (!read_from_io(&sig, 8))
return false;
id = read_chunk(m_chunkIHDR);
if (!(id == id_IHDR && m_chunkIHDR.p.size() == 25))
// 8=HDR+size, 13=size of IHDR chunk, 4=CRC
// http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.IHDR
if (!(id == id_IHDR && m_chunkIHDR.p.size() == 8 + 13 + 4))
{
return false;
}
@ -283,23 +286,25 @@ bool PngDecoder::readHeader()
break;
}
if (id == id_acTL && chunk.p.size() == 20)
if (id == id_acTL)
{
// 8=HDR+size, 8=size of acTL chunk, 4=CRC
// https://wiki.mozilla.org/APNG_Specification#%60acTL%60:_The_Animation_Control_Chunk
if (chunk.p.size() != 8 + 8 + 4)
return false;
m_animation.loop_count = png_get_uint_32(&chunk.p[12]);
if (chunk.p[8] > 0)
{
chunk.p[8] = 0;
chunk.p[9] = 0;
m_frame_count = png_get_uint_32(&chunk.p[8]);
m_frame_count++;
}
else
m_frame_count = png_get_uint_32(&chunk.p[8]);
m_frame_count = png_get_uint_32(&chunk.p[8]);
if (m_frame_count == 0)
return false;
}
if (id == id_fcTL)
{
// 8=HDR+size, 26=size of fcTL chunk, 4=CRC
// https://wiki.mozilla.org/APNG_Specification#%60fcTL%60:_The_Frame_Control_Chunk
if (chunk.p.size() != 8 + 26 + 4)
return false;
m_is_fcTL_loaded = true;
w0 = png_get_uint_32(&chunk.p[12]);
h0 = png_get_uint_32(&chunk.p[16]);
@ -313,6 +318,11 @@ bool PngDecoder::readHeader()
if (id == id_bKGD)
{
// 8=HDR+size, ??=size of bKGD chunk, 4=CRC
// The spec is actually more complex: http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.bKGD
// TODO: we only check that 4 bytes can be read from &chunk.p[8]. Fix.
if (chunk.p.size() < 8 + 4)
return false;
int bgcolor = png_get_uint_32(&chunk.p[8]);
m_animation.bgcolor[3] = (bgcolor >> 24) & 0xFF;
m_animation.bgcolor[2] = (bgcolor >> 16) & 0xFF;
@ -669,34 +679,34 @@ void PngDecoder::compose_frame(std::vector<png_bytep>& rows_dst, const std::vect
});
}
size_t PngDecoder::read_from_io(void* _Buffer, size_t _ElementSize, size_t _ElementCount)
bool PngDecoder::read_from_io(void* buffer, size_t num_bytes)
{
if (m_f)
return fread(_Buffer, _ElementSize, _ElementCount, m_f);
return fread(buffer, 1, num_bytes, m_f) == num_bytes;
if (m_buf_pos + _ElementSize > m_buf.cols * m_buf.rows * m_buf.elemSize()) {
if (m_buf_pos + num_bytes > m_buf.cols * m_buf.rows * m_buf.elemSize()) {
CV_LOG_WARNING(NULL, "PNG input buffer is incomplete");
return 0;
return false;
}
memcpy( _Buffer, m_buf.ptr() + m_buf_pos, _ElementSize );
m_buf_pos += _ElementSize;
return 1;
memcpy( buffer, m_buf.ptr() + m_buf_pos, num_bytes );
m_buf_pos += num_bytes;
return true;
}
uint32_t PngDecoder::read_chunk(Chunk& chunk)
{
unsigned char len[4];
if (read_from_io(&len, 4, 1) == 1)
if (read_from_io(&len, 4))
{
const size_t size = png_get_uint_32(len) + 12;
const size_t size = static_cast<size_t>(png_get_uint_32(len)) + 12;
if (size > PNG_USER_CHUNK_MALLOC_MAX)
{
CV_LOG_WARNING(NULL, "chunk data is too large");
}
chunk.p.resize(size);
memcpy(chunk.p.data(), len, 4);
if (read_from_io(&chunk.p[4], chunk.p.size() - 4, 1) == 1)
if (read_from_io(&chunk.p[4], chunk.p.size() - 4))
return *(uint32_t*)(&chunk.p[4]);
}
return 0;

View File

@ -137,7 +137,7 @@ protected:
bool processing_start(void* frame_ptr, const Mat& img);
bool processing_finish();
void compose_frame(std::vector<png_bytep>& rows_dst, const std::vector<png_bytep>& rows_src, unsigned char bop, uint32_t x, uint32_t y, uint32_t w, uint32_t h, Mat& img);
size_t read_from_io(void* _Buffer, size_t _ElementSize, size_t _ElementCount);
bool read_from_io(void* buffer, size_t num_bytes);
uint32_t read_chunk(Chunk& chunk);
struct PngPtrs {