Skip to content

Heap OOB Read due to wrong indexing #487

Description

@ndaprela

Severity

MEDIUM (CVSS:3.1/AV:L/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N)

Summary

CImg's TIFF loader can read past the end of a decoded tiled TIFF buffer when loading attacker-controlled contiguous tiled images. In _load_tiff_tiled_contig(), the copy loop uses TileLength as the source row stride even though libtiff allocated and filled the tile buffer with TileWidth * TileLength layout, so a narrow tile with a larger tile length makes the subsequent output rows read beyond the decoded tile. This can expose adjacent heap bytes through decoded output or crash hardened builds that detect the out-of-bounds read.

Verified on CImg at commit d1cd9d1b8a0e672801c41a6e922f5a053db559ad (current master).

Details

The public load_tiff() entry point opens the caller-supplied path with TIFFOpen() and then dispatches each selected directory into _load_tiff(), which reads file-controlled geometry and sample metadata from TIFF tags, including SamplesPerPixel, image width, and image length.

For tiled images, it reads TileWidth and TileLength from the same directory and, when the planar configuration is contiguous with 8-bit unsigned samples, dispatches to _load_tiff_tiled_contig<unsigned char>().

That helper allocates a libtiff tile buffer with TIFFTileSize(tif) and fills it with TIFFReadTile().

The copy sink then indexes the decoded tile as:

for (unsigned int rr = row; rr<std::min((unsigned int)(row + th),(unsigned int)ny); ++rr)
    for (unsigned int cc = col; cc<std::min((unsigned int)(col + tw),(unsigned int)nx); ++cc)
        for (unsigned int vv = 0; vv<samplesperpixel; ++vv)
            (*this)(cc,rr,vv) = (T)(ptr[(rr - row)*th*samplesperpixel + (cc - col)*samplesperpixel + vv]);

Since th is TileLength, not the tile row width tw, the pixels bytes are read from the wrong indexes and possibly also accessing heap memory adjacent to the legitimate buffer holding the raw bytes retrieved from the file being processed.

This represents both a functional issue, since this kind of TIFF images will generally be wrongly decoded, and a security issue, since it could allow to access memory out of bounds.

Suggested Remediation

The issue can be fixed by replacing th with tw in the computation of the indexes of pixels:

diff --git a/CImg.h b/CImg.h
index 0c9a9de..99b2178 100644
--- a/CImg.h
+++ b/CImg.h
@@ -56533,7 +56533,7 @@ namespace cimg_library {
             for (unsigned int rr = row; rr<std::min((unsigned int)(row + th),(unsigned int)ny); ++rr)
               for (unsigned int cc = col; cc<std::min((unsigned int)(col + tw),(unsigned int)nx); ++cc)
                 for (unsigned int vv = 0; vv<samplesperpixel; ++vv)
-                  (*this)(cc,rr,vv) = (T)(ptr[(rr - row)*th*samplesperpixel + (cc - col)*samplesperpixel + vv]);
+                  (*this)(cc,rr,vv) = (T)(ptr[(rr - row)*tw*samplesperpixel + (cc - col)*samplesperpixel + vv]);
           }
         _TIFFfree(buf);
       }

Credits

Found by Team Atlanta.
Collected and verified by OSTIF using AI with their own harness.
Manually verified, fixed, and reported by Shielder.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions