potok/opencv
1
0
Fork 0
mirror of https://github.com/opencv/opencv.git synced 2026-07-21 19:33:03 +04:00
Code Packages Releases Wiki Activity

Merge pull request #29296 from arshsmith:fix-pam-grayalpha-overflow

Browse Source 
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.
This commit is contained in:
arshsmith
2026-06-15 20:19:12 +05:30
committed by GitHub
parent 40d6727ea2
commit 6df9732c81
2 changed files with 42 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/imgcodecs/src/grfmt_pam.cpp
+11 -11
View File
@@ -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];
modules/imgcodecs/test/test_grfmt.cpp
+31
View File
@@ -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<uchar> 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<uchar>(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