Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(targa): guard against corrupted tga files #3768

Merged
merged 1 commit into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/targa.imageio/targa_pvt.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ enum tga_alpha_type {
TGA_ALPHA_UNDEFINED_IGNORE = 1, ///< can ignore alpha
TGA_ALPHA_UNDEFINED_RETAIN = 2, ///< undefined, but should be retained
TGA_ALPHA_USEFUL = 3, ///< useful alpha data is present
TGA_ALPHA_PREMULTIPLIED = 4 ///< alpha is pre-multiplied (arrrgh!)
TGA_ALPHA_PREMULTIPLIED = 4, ///< alpha is pre-multiplied (arrrgh!)
TGA_ALPHA_INVALID // one past the last valid value
// values 5-127 are reserved
// values 128-255 are unassigned
};
Expand Down
94 changes: 63 additions & 31 deletions src/targa.imageio/targainput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ class TGAInput final : public ImageInput {
bool readimg();

/// Helper function: decode a pixel.
inline void decode_pixel(unsigned char* in, unsigned char* out,
inline bool decode_pixel(unsigned char* in, unsigned char* out,
unsigned char* palette, int bytespp,
int palbytespp);
int palbytespp, size_t palette_alloc_size);

// Read one byte
bool read(uint8_t& buf) { return ioread(&buf, sizeof(buf), 1); }
Expand All @@ -112,6 +112,21 @@ class TGAInput final : public ImageInput {
swap_endian(&buf);
return ok;
}

// Read maxlen bytes from the file, if it doesn't start with 0, add the
// contents as attribute `name`. Return true if the read was successful,
// false if there was an error reading from the file. The attribute is
// only added if the string is non-empty.
bool read_bytes_for_string_attribute(string_view name, size_t maxlen)
{
char* buf = OIIO_ALLOCA(char, maxlen);
OIIO_DASSERT(buf != nullptr);
if (!ioread(buf, maxlen))
return false;
if (buf[0])
m_spec.attribute(name, Strutil::safe_string_view(buf, maxlen));
return true;
}
};


Expand Down Expand Up @@ -188,19 +203,18 @@ TGAInput::open(const std::string& name, ImageSpec& newspec)
return false;
}

if (is_palette()
&& (m_tga.type == TYPE_GRAY || m_tga.type == TYPE_GRAY_RLE)) {
// it should be an error for TYPE_RGB* as well, but apparently some
// *very* old TGAs can be this way, so we'll hack around it
errorfmt("Palette defined for grayscale image");
return false;
}

if (is_palette()
&& (m_tga.cmap_size != 15 && m_tga.cmap_size != 16
&& m_tga.cmap_size != 24 && m_tga.cmap_size != 32)) {
errorfmt("Illegal palette entry size: {} bits", m_tga.cmap_size);
return false;
if (is_palette()) {
if (m_tga.type == TYPE_GRAY || m_tga.type == TYPE_GRAY_RLE) {
// it should be an error for TYPE_RGB* as well, but apparently some
// *very* old TGAs can be this way, so we'll hack around it
errorfmt("Palette defined for grayscale image");
return false;
}
if (m_tga.cmap_size != 15 && m_tga.cmap_size != 16
&& m_tga.cmap_size != 24 && m_tga.cmap_size != 32) {
errorfmt("Illegal palette entry size: {} bits", m_tga.cmap_size);
return false;
}
}

m_alpha_type = TGA_ALPHA_NONE;
Expand Down Expand Up @@ -324,10 +338,8 @@ TGAInput::read_tga2_header()
} buf;

// load image author
if (!ioread(buf.c, 41, 1))
if (!read_bytes_for_string_attribute("Artist", 41))
return false;
if (buf.c[0])
m_spec.attribute("Artist", (char*)buf.c);

// load image comments
if (!ioread(buf.c, 324, 1))
Expand Down Expand Up @@ -365,10 +377,8 @@ TGAInput::read_tga2_header()
}

// job name/ID
if (!ioread(buf.c, 41, 1))
if (!read_bytes_for_string_attribute("DocumentName", 41))
return false;
if (buf.c[0])
m_spec.attribute("DocumentName", (char*)buf.c);

// job time
if (!ioread(buf.s, 2, 3))
Expand All @@ -390,7 +400,7 @@ TGAInput::read_tga2_header()
return false;
if (buf.c[0]) {
// tack on the version number and letter
std::string soft((const char*)buf.c);
std::string soft = Strutil::safe_string((const char*)buf.c, 41);
soft += Strutil::fmt::format(" {}.{}", n / 100, n % 100);
if (l != ' ')
soft += l;
Expand Down Expand Up @@ -458,6 +468,10 @@ TGAInput::read_tga2_header()
// alpha type
if (!ioread(buf.c, 1, 1))
return false;
if (buf.c[0] >= TGA_ALPHA_INVALID) {
errorfmt("Invalid alpha type {}. Corrupted header?", (int)buf.c[0]);
return false;
}
m_alpha_type = (tga_alpha_type)buf.c[0];
if (m_alpha_type)
m_spec.attribute("targa:alpha_type", m_alpha_type);
Expand Down Expand Up @@ -522,11 +536,13 @@ TGAInput::get_thumbnail(ImageBuf& thumb, int subimage)
alphabits = 8;
// read palette, if there is any
std::unique_ptr<unsigned char[]> palette;
size_t palette_alloc_size = 0;
if (is_palette()) {
if (!ioseek(m_ofs_palette)) {
return false;
}
palette.reset(new unsigned char[palbytespp * m_tga.cmap_length]);
palette_alloc_size = palbytespp * m_tga.cmap_length;
palette.reset(new unsigned char[palette_alloc_size]);
if (!ioread(palette.get(), palbytespp, m_tga.cmap_length))
return false;
if (!ioseek(m_ofs_thumb + 2)) {
Expand All @@ -542,7 +558,9 @@ TGAInput::get_thumbnail(ImageBuf& thumb, int subimage)
x++, img += m_spec.nchannels) {
if (!ioread(in, bytespp, 1))
return false;
decode_pixel(in, pixel, palette.get(), bytespp, palbytespp);
if (!decode_pixel(in, pixel, palette.get(), bytespp, palbytespp,
palette_alloc_size))
return false;
memcpy(img, pixel, m_spec.nchannels);
}
}
Expand All @@ -557,9 +575,10 @@ TGAInput::get_thumbnail(ImageBuf& thumb, int subimage)



inline void
inline bool
TGAInput::decode_pixel(unsigned char* in, unsigned char* out,
unsigned char* palette, int bytespp, int palbytespp)
unsigned char* palette, int bytespp, int palbytespp,
size_t palette_alloc_size)
{
unsigned int k = 0;
// I hate nested switches...
Expand All @@ -569,6 +588,10 @@ TGAInput::decode_pixel(unsigned char* in, unsigned char* out,
for (int i = 0; i < bytespp; ++i)
k |= in[i] << (8 * i); // Assemble it in little endian order
k = (m_tga.cmap_first + k) * palbytespp;
if (k + palbytespp > palette_alloc_size) {
errorfmt("Corrupt palette index");
return false;
}
switch (palbytespp) {
case 2:
// see the comment for 16bpp RGB below for an explanation of this
Expand Down Expand Up @@ -643,6 +666,7 @@ TGAInput::decode_pixel(unsigned char* in, unsigned char* out,
memcpy(out, in, bytespp);
break;
}
return true;
}


Expand Down Expand Up @@ -710,8 +734,10 @@ TGAInput::readimg()

// read palette, if there is any
std::unique_ptr<unsigned char[]> palette;
size_t palette_alloc_size = 0;
if (is_palette()) {
palette.reset(new unsigned char[palbytespp * m_tga.cmap_length]);
palette_alloc_size = palbytespp * m_tga.cmap_length;
palette.reset(new unsigned char[palette_alloc_size]);
if (!ioread(palette.get(), palbytespp, m_tga.cmap_length))
return false;
}
Expand All @@ -725,7 +751,9 @@ TGAInput::readimg()
for (int64_t x = 0; x < m_spec.width; x++) {
if (!ioread(in, bytespp, 1))
return false;
decode_pixel(in, pixel, palette.get(), bytespp, palbytespp);
if (!decode_pixel(in, pixel, palette.get(), bytespp, palbytespp,
palette_alloc_size))
return false;
memcpy(m_buf.get() + y * m_spec.width * m_spec.nchannels
+ x * m_spec.nchannels,
pixel, m_spec.nchannels);
Expand All @@ -743,7 +771,9 @@ TGAInput::readimg()
return false;
}
packet_size = 1 + (in[0] & 0x7f);
decode_pixel(&in[1], pixel, palette.get(), bytespp, palbytespp);
if (!decode_pixel(&in[1], pixel, palette.get(), bytespp,
palbytespp, palette_alloc_size))
return false;
if (in[0] & 0x80) { // run length packet
// DBG("[tga] run length packet size {} @ ({},{})\n",
// packet_size, x, y);
Expand Down Expand Up @@ -786,8 +816,10 @@ TGAInput::readimg()
DBG("Failed on scanline {}\n", y);
return false;
}
decode_pixel(&in[1], pixel, palette.get(), bytespp,
palbytespp);
if (!decode_pixel(&in[1], pixel, palette.get(),
bytespp, palbytespp,
palette_alloc_size))
return false;
// DBG("\t\t@ ({},{}): [{:d} {:d} {:d} {:d}]\n", x, y,
// pixel[0], pixel[1], pixel[2], pixel[3]);
}
Expand Down
6 changes: 6 additions & 0 deletions testsuite/targa/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,12 @@ Full command line was:
oiiotool ERROR: read : "src/crash6.tga": Read error: hit end of file
Full command line was:
> oiiotool --oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr
oiiotool ERROR: read : "src/crash1707.tga": Invalid alpha type 138. Corrupted header?
Full command line was:
> oiiotool --oiioattrib try_all_readers 0 src/crash1707.tga -o crash1707.exr
oiiotool ERROR: read : "src/crash1708.tga": Corrupt palette index
Full command line was:
> oiiotool --oiioattrib try_all_readers 0 src/crash1708.tga -o crash1707.exr
Reading src/1x1.tga
src/1x1.tga : 1 x 1, 3 channel, uint2 targa
SHA-1: DB9237F28F9622F7B7E73B783A8822B63BDB3708
Expand Down
2 changes: 2 additions & 0 deletions testsuite/targa/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
command += oiiotool("--oiioattrib try_all_readers 0 src/crash4.tga -o crash4.exr", failureok = True)
command += oiiotool("--oiioattrib try_all_readers 0 src/crash5.tga -o crash5.exr", failureok = True)
command += oiiotool("--oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr", failureok = True)
command += oiiotool("--oiioattrib try_all_readers 0 src/crash1707.tga -o crash1707.exr", failureok = True)
command += oiiotool("--oiioattrib try_all_readers 0 src/crash1708.tga -o crash1707.exr", failureok = True)

# Test odds and ends, unusual files
command += rw_command("src", "1x1.tga")
Expand Down
Binary file added testsuite/targa/src/crash1707.tga
Binary file not shown.
Binary file added testsuite/targa/src/crash1708.tga
Binary file not shown.