Skip to content

Commit

Permalink
BMP output safety (AcademySoftwareFoundation#3673)
Browse files Browse the repository at this point in the history
In open(), check for non-zero image offsets, which is not supported by
BMP, and issue an error if found. If the spec passed in has nonzero
spec.y, other code in the write functions would fail because they
assumed spec.y == 0 and could therefore access memory incorrectly.

More generally, also put checks in the write functions that issue
errors and take early outs if they find that they are called on a
BMPOutput that is not currently open, for example if open() fails but
the user doesn't check the return codes and proceeds to call the write
functions anyway.

This addresses TALOS-2022-1653 / CVE-2022-43594-CVE-2022-43595
  • Loading branch information
lgritz committed Nov 19, 2022
1 parent 1413022 commit 3a71e8a
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/bmp.imageio/bmpoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ BmpOutput::open(const std::string& name, const ImageSpec& spec, OpenMode mode)
return false;
}

if (m_spec.x || m_spec.y || m_spec.z) {
errorfmt("{} does not support images with non-zero image origin offset",
format_name());
return false;
}

// Only support 8 bit channels for now.
m_spec.set_format(TypeDesc::UINT8);
m_dither = m_spec.get_int_attribute("oiio:dither", 0);
Expand Down Expand Up @@ -133,12 +139,18 @@ bool
BmpOutput::write_scanline(int y, int z, TypeDesc format, const void* data,
stride_t xstride)
{
if (y > m_spec.height) {
if (!ioproxy_opened()) {
errorfmt("write_scanline called but file is not open.");
return false;
}

if (y > m_spec.y + m_spec.height) {
errorfmt("Attempt to write too many scanlines to {}", m_filename);
close();
return false;
}

y -= m_spec.y;
if (m_spec.width >= 0)
y = (m_spec.height - y - 1);
int64_t scanline_off = y * m_padded_scanline_size;
Expand All @@ -165,9 +177,14 @@ bool
BmpOutput::write_tile(int x, int y, int z, TypeDesc format, const void* data,
stride_t xstride, stride_t ystride, stride_t zstride)
{
if (!ioproxy_opened()) {
errorfmt("write_tile called but file is not open.");
return false;
}

// Emulate tiles by buffering the whole image
return copy_tile_to_image_buffer(x, y, z, format, data, xstride, ystride,
zstride, &m_tilebuffer[0]);
zstride, m_tilebuffer.data());
}


Expand All @@ -181,11 +198,11 @@ BmpOutput::close(void)
}

bool ok = true;
if (m_spec.tile_width) {
if (m_spec.tile_width && m_tilebuffer.size()) {
// Handle tile emulation -- output the buffered pixels
OIIO_DASSERT(m_tilebuffer.size());
ok &= write_scanlines(m_spec.y, m_spec.y + m_spec.height, 0,
m_spec.format, &m_tilebuffer[0]);
m_spec.format, m_tilebuffer.data());
std::vector<unsigned char>().swap(m_tilebuffer);
}

Expand Down

0 comments on commit 3a71e8a

Please sign in to comment.