From 6df9732c81a2774cee2045d743fb76ab9b9925de Mon Sep 17 00:00:00 2001 From: arshsmith Date: Mon, 15 Jun 2026 20:19:12 +0530 Subject: [PATCH] Merge pull request #29296 from arshsmith:fix-pam-grayalpha-overflow imgcodecs(pam): fix out-of-bounds write when reading 2-channel PAM #29296 While poking at the PAM decoder I noticed basic_conversion() writes past the end of the destination buffer when a GRAYSCALE_ALPHA (2 channel) image is read with IMREAD_GRAYSCALE. The 1-channel branch was written as if the destination had 3 channels: for( ; s < end; d += 3, s += src_sampe_size ) d[0] = d[1] = d[2] = s[layout->graychan]; So for every source pixel it writes 3 bytes and advances d by 3, even though the output row only has m_width bytes (1 channel). With m_channels == 2 that ends up writing ~1.5 * m_width bytes per row, which runs off the row and, on the last row, off the end of the Mat. m_width is taken straight from the file header (up to 2^20), so the overflow size and contents are attacker controlled. It's reachable with a plain imread(file, IMREAD_GRAYSCALE) on a crafted file. While looking at it I also realised the loop bound was off for any multi channel source: end was set to src + src_width, but the source has src_width * channels samples, so it only ever processed m_width/channels pixels instead of all of them. That's why GRAYSCALE_ALPHA / RGB_ALPHA come out only partially filled. Fix both at once: - end now covers the whole row (src_width * src_sampe_size) - the 1-channel branch writes one byte and advances d by 1 For the common grayscale->color case (channels == 1) the new end is identical to the old one (m_width * 1 == m_width), so that path is byte for byte the same and the existing PAM read_write test is unaffected. The only outputs that change are the GRAYSCALE_ALPHA/RGB_ALPHA conversions, which were already broken. Added a regression test (Imgcodecs_Pam.decode_graya_as_gray) that builds a small 2-channel PAM with an odd width, decodes it as grayscale and checks the result matches the gray channel. It overflows/crashes on the old code and passes now. --- modules/imgcodecs/src/grfmt_pam.cpp | 22 +++++++++---------- modules/imgcodecs/test/test_grfmt.cpp | 31 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/modules/imgcodecs/src/grfmt_pam.cpp b/modules/imgcodecs/src/grfmt_pam.cpp index efae09f033..abc48cd1ad 100644 --- a/modules/imgcodecs/src/grfmt_pam.cpp +++ b/modules/imgcodecs/src/grfmt_pam.cpp @@ -175,28 +175,28 @@ rgb_convert (void *src, void *target, int width, int target_channels, int target */ static void -basic_conversion (void *src, const struct channel_layout *layout, int src_sampe_size, +basic_conversion (void *src, const struct channel_layout *layout, int src_sample_size, int src_width, void *target, int target_channels, int target_depth, bool use_rgb) { switch (target_depth) { case CV_8U: { uchar *d = (uchar *)target, *s = (uchar *)src, - *end = ((uchar *)src) + src_width; + *end = ((uchar *)src) + src_width * src_sample_size; switch (target_channels) { case 1: - for( ; s < end; d += 3, s += src_sampe_size ) - d[0] = d[1] = d[2] = s[layout->graychan]; + for( ; s < end; d += 1, s += src_sample_size ) + d[0] = s[layout->graychan]; break; case 3: if (use_rgb) - for( ; s < end; d += 3, s += src_sampe_size ) { + for( ; s < end; d += 3, s += src_sample_size ) { d[0] = s[layout->rchan]; d[1] = s[layout->gchan]; d[2] = s[layout->bchan]; } else - for( ; s < end; d += 3, s += src_sampe_size ) { + for( ; s < end; d += 3, s += src_sample_size ) { d[0] = s[layout->bchan]; d[1] = s[layout->gchan]; d[2] = s[layout->rchan]; @@ -210,21 +210,21 @@ basic_conversion (void *src, const struct channel_layout *layout, int src_sampe_ case CV_16U: { ushort *d = (ushort *)target, *s = (ushort *)src, - *end = ((ushort *)src) + src_width; + *end = ((ushort *)src) + src_width * src_sample_size; switch (target_channels) { case 1: - for( ; s < end; d += 3, s += src_sampe_size ) - d[0] = d[1] = d[2] = s[layout->graychan]; + for( ; s < end; d += 1, s += src_sample_size ) + d[0] = s[layout->graychan]; break; case 3: if (use_rgb) - for( ; s < end; d += 3, s += src_sampe_size ) { + for( ; s < end; d += 3, s += src_sample_size ) { d[0] = s[layout->rchan]; d[1] = s[layout->gchan]; d[2] = s[layout->bchan]; } else - for( ; s < end; d += 3, s += src_sampe_size ) { + for( ; s < end; d += 3, s += src_sample_size ) { d[0] = s[layout->bchan]; d[1] = s[layout->gchan]; d[2] = s[layout->rchan]; diff --git a/modules/imgcodecs/test/test_grfmt.cpp b/modules/imgcodecs/test/test_grfmt.cpp index 87d8780442..bc31cc3c6a 100644 --- a/modules/imgcodecs/test/test_grfmt.cpp +++ b/modules/imgcodecs/test/test_grfmt.cpp @@ -564,6 +564,37 @@ TEST(Imgcodecs_Pam, read_write) remove(writefile.c_str()); remove(writefile_no_param.c_str()); } + +// Regression test: a 2-channel (GRAYSCALE_ALPHA) PAM decoded as single channel +// used to overflow the output row in basic_conversion() (3 bytes written per +// source pixel into a 1-channel row). Verify it decodes safely and correctly. +TEST(Imgcodecs_Pam, decode_graya_as_gray) +{ + const int width = 9, height = 3; // odd width to expose off-by-row overflow + std::string header = cv::format( + "P7\nWIDTH %d\nHEIGHT %d\nDEPTH 2\nMAXVAL 255\n" + "TUPLTYPE GRAYSCALE_ALPHA\nENDHDR\n", width, height); + + std::vector buf(header.begin(), header.end()); + Mat gray_ref(height, width, CV_8UC1); + for (int y = 0; y < height; y++) + for (int x = 0; x < width; x++) + { + uchar gray = (uchar)((y * width + x) * 7 + 1); + uchar alpha = (uchar)(255 - gray); + gray_ref.at(y, x) = gray; + buf.push_back(gray); // channel 0: gray + buf.push_back(alpha); // channel 1: alpha (must be ignored) + } + + Mat decoded; + ASSERT_NO_THROW(decoded = imdecode(buf, IMREAD_GRAYSCALE)); + ASSERT_FALSE(decoded.empty()); + EXPECT_EQ(width, decoded.cols); + EXPECT_EQ(height, decoded.rows); + EXPECT_EQ(1, decoded.channels()); + EXPECT_EQ(0, cvtest::norm(gray_ref, decoded, NORM_INF)); +} #endif #ifdef HAVE_IMGCODEC_PFM