Skip to content

Commit

Permalink
BMP: protect against corrupt pixel coordinates
Browse files Browse the repository at this point in the history
When reading the image (which can have signals to jump around in the
array), we checked for out-of-bounds x coordinate, but not y, and some
corrupted files could overrun buffers as a result.
  • Loading branch information
lgritz committed Oct 21, 2022
1 parent 3a9d3fa commit 625b56e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 5 deletions.
23 changes: 19 additions & 4 deletions src/bmp.imageio/bmpinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,18 @@ BmpInput::read_rle_image()
bool ok = true;
int y = 0, x = 0;
while (ok) {
// Strutil::print("currently at {},{}\n", x, y);
unsigned char rle_pair[2];
if (!ioread(rle_pair, 2)) {
ok = false;
// Strutil::print("hit end of file at {},{}\n", x, y);
break;
}
if (y >= m_spec.height) { // out of y bounds
errorfmt(
"BMP might be corrupted, it is referencing an out-of-bounds pixel coordinte ({},{})",
x, y);
ok = false;
break;
}
int npixels = rle_pair[0];
Expand All @@ -294,8 +303,10 @@ BmpInput::read_rle_image()
// [0,0] is end of line marker
x = 0;
++y;
// Strutil::print("end of line, moving to {},{}\n", x, y);
} else if (npixels == 0 && value == 1) {
// [0,1] is end of bitmap marker
// Strutil::print("end of bitmap\n");
break;
} else if (npixels == 0 && value == 2) {
// [0,2] is a "delta" -- two more bytes reposition the
Expand All @@ -304,14 +315,17 @@ BmpInput::read_rle_image()
ok &= ioread(offset, 2);
x += offset[0];
y += offset[1];
// Strutil::print("offset by {:d},{:d} to {},{}\n", offset[0],
// offset[1], x, y);
} else if (npixels == 0) {
// [0,n>2] is an "absolute" run of pixel data.
// n is the number of pixel indices that follow, but note
// that it pads to word size.
int npixels = value;
int nbytes = (rletype == 4)
? round_to_multiple((npixels + 1) / 2, 2)
: round_to_multiple(npixels, 2);
npixels = value;
int nbytes = (rletype == 4)
? round_to_multiple((npixels + 1) / 2, 2)
: round_to_multiple(npixels, 2);
// Strutil::print("rle of {} pixels at {},{}\n", npixels, x, y);
unsigned char absolute[256];
ok &= ioread(absolute, nbytes);
for (int i = 0; i < npixels; ++i, ++x) {
Expand All @@ -325,6 +339,7 @@ BmpInput::read_rle_image()
}
} else {
// [n>0,p] is a run of n pixels.
// Strutil::print("direct read {} pixels at {},{}\n", npixels, x, y);
for (int i = 0; i < npixels; ++i, ++x) {
int v;
if (rletype == 4)
Expand Down
4 changes: 4 additions & 0 deletions testsuite/bmp/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,7 @@ PASS
oiiotool ERROR: read : "src/decodecolormap-corrupt.bmp": Possible corrupted header, invalid palette size
Full command line was:
> oiiotool --info -v -a --hash src/decodecolormap-corrupt.bmp
oiiotool ERROR: read : "src/bad-y.bmp": BMP might be corrupted, it is referencing an out-of-bounds pixel coordinte (0,255)
BMP error reading rle-compressed image
Full command line was:
> oiiotool --info -v -a --hash src/bad-y.bmp
3 changes: 2 additions & 1 deletion testsuite/bmp/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@
# Test BMP of the 56-byte DIB header variety
command += rw_command ("../oiio-images/bmp", "gracehopper.bmp")

# See if we handle this corrupt file with a useful error message
# See if we handle these corrupt files with useful error messages
command += info_command ("src/decodecolormap-corrupt.bmp")
command += info_command ("src/bad-y.bmp")
Binary file added testsuite/bmp/src/bad-y.bmp
Binary file not shown.

0 comments on commit 625b56e

Please sign in to comment.