Skip to content

Commit

Permalink
IFF output safety (AcademySoftwareFoundation#3676)
Browse files Browse the repository at this point in the history 
* Maya IFF documentation says IFF file has max resolution of 8192, and
  our implementation only allows writing RGB and RGBA, so check these
  limits when an IFF file is opened for output.

* Make sure scratch space allocates enough for tile padding.

* Check for non-zero image origin coordinates (which are not allowed
  in IFF files).

* Change some 16-bit loop variables to uint32_t to avoid possible
  overflow.

Fixes TALOS-2022-1654, TALOS-2022-1655, TALOS-2022-1656
  • Loading branch information
@lgritz
lgritz committed Nov 19, 2022
1 parent c18b3ef commit 99f17b8
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 43 deletions.
10 changes: 4 additions & 6 deletions src/iff.imageio/iff_pvt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,17 @@ align_size(uint32_t size, uint32_t alignment)
}

// tile width
inline const int&
constexpr uint32_t
tile_width()
{
static int tile_w = 64;
return tile_w;
return 64;
}

// tile height
inline const int&
constexpr uint32_t
tile_height()
{
static int tile_h = 64;
return tile_h;
return 64;
}

// tile width size
Expand Down
93 changes: 56 additions & 37 deletions src/iff.imageio/iffoutput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ int
IffOutput::supports(string_view feature) const
{
return (feature == "tiles" || feature == "alpha" || feature == "nchannels"
|| feature == "ioproxy");
|| feature == "ioproxy" || feature == "origin");
}


Expand Down Expand Up @@ -142,16 +142,37 @@ IffOutput::open(const std::string& name, const ImageSpec& spec, OpenMode mode)
return false;
}

// close(); // Close any already-opened file
// saving 'name' and 'spec' for later use
m_filename = name;
m_spec = spec;

// Maya docs say 8k is the limit
if (m_spec.width < 1 || m_spec.width > 8192 || m_spec.height < 1
|| m_spec.height > 8192) {
errorfmt("Image resolution {} x {} is not valid for an IFF file",
m_spec.width, m_spec.height);
return false;
}
// This implementation only supports writing RGB and RGBA images as IFF
if (m_spec.nchannels < 3 || m_spec.nchannels > 4) {
errorfmt("Cannot write IFF file with {} channels", m_spec.nchannels);
return false;
}

// tiles
m_spec.tile_width = tile_width();
m_spec.tile_height = tile_height();
m_spec.tile_depth = 1;

uint64_t xtiles = tile_width_size(m_spec.width);
uint64_t ytiles = tile_height_size(m_spec.height);
if (xtiles * ytiles >= (1 << 16)) { // The format can't store it!
errorfmt(
"Too high a resolution ({}x{}), exceeds maximum of 64k tiles in the image\n",
m_spec.width, m_spec.height);
return false;
}

ioproxy_retrieve_from_config(m_spec);
if (!ioproxy_use_or_open(name))
return false;
Expand All @@ -172,16 +193,6 @@ IffOutput::open(const std::string& name, const ImageSpec& spec, OpenMode mode)
m_iff_header.compression
= (m_spec.get_string_attribute("compression") == "none") ? NONE : RLE;

uint64_t xtiles = tile_width_size(m_spec.width);
uint64_t ytiles = tile_height_size(m_spec.height);
if (xtiles * ytiles >= (1 << 16)) { // The format can't store it!
errorfmt(
"Too high a resolution ({}x{}), exceeds maximum of 64k tiles in the image\n",
m_spec.width, m_spec.height);
close();
return false;
}

// we write the header of the file
m_iff_header.x = m_spec.x;
m_iff_header.y = m_spec.y;
Expand Down Expand Up @@ -274,6 +285,11 @@ bool
IffOutput::write_scanline(int y, int z, TypeDesc format, const void* data,
stride_t xstride)
{
if (!ioproxy_opened()) {
errorfmt("write_scanline called but file is not open.");
return false;
}

// scanline not used for Maya IFF, uses tiles instead.
// Emulate by copying the scanline to the buffer we're accumulating.
std::vector<unsigned char> scratch;
Expand All @@ -291,6 +307,11 @@ bool
IffOutput::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;
}

// auto stride
m_spec.auto_stride(xstride, ystride, zstride, format, spec().nchannels,
spec().tile_width, spec().tile_height);
Expand Down Expand Up @@ -353,15 +374,13 @@ IffOutput::close(void)
uint8_t channels = m_iff_header.pixel_channels;

// set tile coordinates
uint16_t xmin, xmax, ymin, ymax;

// set xmin and xmax
xmin = tx * tile_width();
xmax = std::min(xmin + tile_width(), m_spec.width) - 1;

// set ymin and ymax
ymin = ty * tile_height();
ymax = std::min(ymin + tile_height(), m_spec.height) - 1;
uint32_t xmin = tx * tile_width();
uint32_t xmax
= std::min(xmin + tile_width(), uint32_t(m_spec.width)) - 1;
uint32_t ymin = ty * tile_height();
uint32_t ymax = std::min(ymin + tile_height(),
uint32_t(m_spec.height))
- 1;

// set width and height
uint32_t tw = xmax - xmin + 1;
Expand Down Expand Up @@ -408,13 +427,11 @@ IffOutput::close(void)
uint8_t* in_p = &in[0];

// set tile
for (uint16_t py = ymin; py <= ymax; py++) {
for (uint32_t py = ymin; py <= ymax; py++) {
const uint8_t* in_dy
= &m_buf[0]
+ (py * m_spec.width)
* m_spec.pixel_bytes();
= &m_buf[0] + py * m_spec.scanline_bytes();

for (uint16_t px = xmin; px <= xmax; px++) {
for (uint32_t px = xmin; px <= xmax; px++) {
// get pixel
uint8_t pixel;
const uint8_t* in_dx
Expand Down Expand Up @@ -445,6 +462,8 @@ IffOutput::close(void)
// set length
uint32_t align = align_size(length, 4);
if (align > length) {
if (scratch.size() < index + align - length)
scratch.resize(index + align - length);
out_p = &scratch[0] + index;
// Pad.
for (uint32_t i = 0; i < align - length; i++) {
Expand All @@ -457,12 +476,12 @@ IffOutput::close(void)
}
}
if (!tile_compress) {
for (uint16_t py = ymin; py <= ymax; py++) {
for (uint32_t py = ymin; py <= ymax; py++) {
const uint8_t* in_dy = &m_buf[0]
+ (py * m_spec.width)
* m_spec.pixel_bytes();

for (uint16_t px = xmin; px <= xmax; px++) {
for (uint32_t px = xmin; px <= xmax; px++) {
// Map: RGB(A)8 RGBA to BGRA
for (int c = channels - 1; c >= 0; --c) {
// get pixel
Expand Down Expand Up @@ -517,13 +536,11 @@ IffOutput::close(void)
uint8_t* in_p = &in[0];

// set tile
for (uint16_t py = ymin; py <= ymax; py++) {
for (uint32_t py = ymin; py <= ymax; py++) {
const uint8_t* in_dy
= &m_buf[0]
+ (py * m_spec.width)
* m_spec.pixel_bytes();
= &m_buf[0] + py * m_spec.scanline_bytes();

for (uint16_t px = xmin; px <= xmax; px++) {
for (uint32_t px = xmin; px <= xmax; px++) {
// get pixel
uint8_t pixel;
const uint8_t* in_dx
Expand Down Expand Up @@ -555,6 +572,8 @@ IffOutput::close(void)
// set length
uint32_t align = align_size(length, 4);
if (align > length) {
if (scratch.size() < index + align - length)
scratch.resize(index + align - length);
out_p = &scratch[0] + index;
// Pad.
for (uint32_t i = 0; i < align - length; i++) {
Expand All @@ -568,12 +587,12 @@ IffOutput::close(void)
}

if (!tile_compress) {
for (uint16_t py = ymin; py <= ymax; py++) {
for (uint32_t py = ymin; py <= ymax; py++) {
const uint8_t* in_dy = &m_buf[0]
+ (py * m_spec.width)
* m_spec.pixel_bytes();

for (uint16_t px = xmin; px <= xmax; px++) {
for (uint32_t px = xmin; px <= xmax; px++) {
// map: RGB(A) to BGRA
for (int c = channels - 1; c >= 0; --c) {
uint16_t pixel;
Expand All @@ -597,8 +616,8 @@ IffOutput::close(void)
return false;

// write xmin, xmax, ymin and ymax
if (!write(&xmin) || !write(&ymin) || !write(&xmax)
|| !write(&ymax))
if (!write_short(xmin) || !write_short(ymin)
|| !write_short(xmax) || !write_short(ymax))
return false;

// write tile
Expand Down

0 comments on commit 99f17b8

Please sign in to comment.