bug: get_image_info returns true after copying an uninitialised buffer into the caller's output when every UDIM tile is unreadable

Leggi il ramo UDIM in src/libtexture/imagecache.cpp, in particolare il flag first e il memcpy finale con il valore restituito. Conferma il comportamento con una chiamata diretta a ImageCache::get_image_info su un set UDIM in cui nessuna tile si apre; l’issue include un riproduttore e i risultati attesi. Il risultato atteso è che la chiamata restituisca false senza sovrascrivere l’output del chiamante quando nessuna tile ha fornito un valore.

Describe the bug

ImageCacheImpl::get_image_info aggregates an attribute across the tiles of a
UDIM set. If every tile it finds is unreadable, it copies an internal buffer
that nothing ever wrote into the caller's output and returns true. The
caller's variable is overwritten, with whatever the stack held.

src/libtexture/imagecache.cpp, at main:

    if (file->is_udim()) {                                        // :3111
        size_t size  = datatype.size();                           // :3115
        char* common = OIIO_ALLOCA(char, size);                   // :3116
        char* single = OIIO_ALLOCA(char, size);                   // :3117
        bool first   = true;                                      // :3118
        ...
            if (concretefile && !concretefile->broken()) {        // :3128
                bool r = get_image_info(concretefile, ...,        // :3132
                                        first ? common : single);
                if (!r) {
                    // If we couldn't retrieve the attribute from at
                    // least one existing file, it's a failure.
                    (void)geterror();                             // :3138
                    return false;                                 // :3139
                }
                if (first) {
                    first = false;                                // :3142
                ...
        }
        // If we got this far, all the tiles we found had the attribute, and
        // they all had the same value. Copy to the user and return success.
        memcpy(data, common, size);                               // :3153
        return true;                                              // :3154
    }

Note that the inner failure path at :3135-3140 is correct: a tile that opens
but cannot answer the query fails the whole call. The defect is the other path.
OIIO_ALLOCA does not initialise its buffer, and the guard at :3128 has no
else, so a tile that is already broken is skipped in silence. When every tile
is skipped that way the inner get_image_info is never called, first stays
true, common is never written, and :3153 copies it to the caller under a
true return. The comment at :3151 states the postcondition the code does
not establish: there were no tiles that had the attribute. The one-line version
of the defect is that the closing memcpy and return true are not predicated
on !first.

Every tile in the set has to fail, and it has to fail at open rather than
later. UDIM sets are inventoried by directory listing and name matching alone,
so any file sitting at a UDIM index joins the set regardless of its contents,
and a zero-byte, wrong-format or header-damaged one joins it and then fails to
open. A file whose header survives does open, so a write truncated after the
header is complete does not show this.

Evidence

Every tile below comes from the public corpora rather than from a file written
for the occasion. The damaged tiles are OpenEXR's own fuzzer corpus,
openexr-images/Damaged/, whose files carry the real EXR magic and fail during
header parse, not at format detection. The readable tile is
openexr-images/DisplayWindow/t01.exr, 400 x 300, 3 channel half. The corpus
files ship without a .exr extension, so each was copied to a tile name first.

Queried through ImageCache::get_image_info, with 0x5A5A5A5A in the caller's
int and a string literal in the caller's const char* beforehand:

tile set	subimages	fileformat
one damaged tile, asan_heap-oob_504235_131_…	true, wrote -701499600	true, pointer …D62FF790
two damaged tiles, different files	true, wrote 769522064	true, pointer …2DDDF9F0
one tile of 30 ASCII bytes	true, wrote -659911935	true, pointer …C96FFD80
one zero-byte tile	true, wrote 357628192	true, pointer …1550F980
one tile truncated to 200 bytes	true, wrote -1464863040	true, pointer …A8AFF720
the same damaged tile, queried by its concrete name	false, sentinel intact	false, sentinel intact
one readable tile	true, wrote 1	true, "openexr"
readable tile at 1001, damaged tile at 1002	true, wrote 1	true, "openexr"

The last three rows are the controls. Queried by its concrete name the damaged
file returns false and leaves the caller's variable at the sentinel, which is
the correct behaviour and the contrast that matters. A set with anything
readable in it answers correctly, whichever index the readable tile sits at.
Only when every tile fails to open does the aggregate report success.

The reason is in geterror() in the failing cases, but a caller has no reason
to look after a true return.

What reaches the caller is an address, not a zero

The value is neither zero nor stable. Six fresh processes on one fixture:

subimages returned=1 value=615511760    fileformat pointer=0000001C24AFF730
subimages returned=1 value=-1768883216  fileformat pointer=000000829690FC50
subimages returned=1 value=-45090496    fileformat pointer=0000009CFD4FF9A0
subimages returned=1 value=-751829952   fileformat pointer=00000042D32FFCA0
subimages returned=1 value=-674236144   fileformat pointer=0000005ED7CFF910
subimages returned=1 value=1096611904   fileformat pointer=000000B8415CF8A0

The int is consistently 0x60 below the pointer, and both lie inside the
caller's own frame. Recording the address of a local in main() and comparing
against it:

subimages  value=-701499600    -> main()'s stack, anchor-192 bytes
fileformat pointer=…D62FF790   -> main()'s stack, anchor-96 bytes

So what the caller gets back is a live address out of its own stack, differing
run to run with ASLR. On 3.2.0.2dev and 3.1.12 the fileformat pointer is a
heap address instead.

fileformat is the sharpest case. It is a TypeString query, so the caller
passes the address of a const char* and is expected to dereference what comes
back. A caller that checks the pointer for null before dereferencing passes
that check and then reads an arbitrary location.

Any attribute name reaches it, including one that is not supported

channels returns the same uninitialised value. So does udim, which the
implementation does not handle at all: imagecache.h:613-614 documents the
name in lowercase and imagecache.cpp:72 interns "UDIM".

every tile unreadable:  udim  returned=1, wrote -701499600
readable set:           udim  returned=0, sentinel intact

On a working set the unsupported name correctly returns false. On an
all-unreadable set it returns success with the uninitialised buffer, so the
aggregate answers a question it does not recognise.

exists and broken are resolved before this branch and are unaffected,
though they sit oddly beside the rest: on a set whose every tile is unreadable,
exists returns 1 and broken returns 0.

OpenImageIO version and dependencies

OIIO 3.2.2.0 | Windows/x86_64
    Build compiler: MSVS 1951 | C++17/199711
    HW features enabled at build: sse2,sse3,ssse3,sse41,sse42
    No CUDA support (disabled / unavailable at build time)
Dependencies: BZip2 1.0.8, DCMTK NONE, FFmpeg NONE, fmt 12.1.0, Freetype 2.13.3, GIF 5.2.2, Imath 3.2.2, JXL NONE, libdeflate 1.25, Libheif NONE, libjpeg-turbo 3.1.3, LibRaw NONE, libuhdr NONE, OpenColorIO 2.5.1, OpenCV NONE, OpenEXR 3.4.7, OpenJPEG 2.5.4, openjph 0.26.3, PNG 1.6.55, Ptex NONE, Ptex NONE, Robinmap 1.4.1, TBB NONE, TIFF 4.7.1, WebP 1.6.0, ZLIB 1.3.1, zstd 1.5.7

The figures in the evidence above come from a source build of main at
3e8576595, so that the line numbers cited and the behaviour measured are the
same tree:

OIIO 3.3.0.0dev | Windows/x86_64
    Build compiler: MSVS 1951 | C++17/199711
Dependencies: OpenEXR 3.4.7, Imath 3.2.2, fmt 12.1.0, libdeflate 1.25, ZLIB 1.3.1

That build was configured with OIIO_BUILD_TESTS=ON, USE_PYTHON=OFF,
STOP_ON_WARNING=OFF and OIIO_HARDENING=1, Release. None of those bear on
this: there is no assertion involved, only an uninitialised buffer that is
copied out.

The same fixture behaves identically on 3.1.12, on 3.2.0.2dev and on main
at 3e8576595: returned=1 with a varying nonzero value in every case.

oiiotool crashes on it, and only on it. oiiotool --info 'tex.<UDIM>.exr'
prints nothing and terminates with 0xC0000409 when every tile is unreadable.
Every other UDIM case exits 1 with a clear message, which isolates the crash to
this defect rather than to UDIM handling in general:

tiles                                    exit         output
all unreadable                           0xC0000409   none
one readable tile                        1            Cannot retrieve ImageSpec of a UDIM-like virtual file
one tile truncated after its header      1            Cannot retrieve ImageSpec of a UDIM-like virtual file
one readable tile plus one unreadable    1            ... EXR_ERR_FILE_BAD_HEADER ... tex.1002.exr

The partial-set row is worth noting: there oiiotool names the offending tile.

A cacheless ImageBuf does not reach the aggregate, because it opens through
ImageInput, which cannot open a UDIM pattern. The clean library-level
demonstration is a direct ImageCache caller.

A cache-backed ImageBuf issues exactly the subimages, miplevels and
fileformat queries above, but its init_spec then returns false for a
separate reason, "Cannot retrieve ImageSpec of a UDIM-like virtual file",
which fires on a readable set too. What does surface on the ImageBuf is the
uninitialised value itself:

nsubimages()       -> 1483733088
file_format_name() -> "`\xefoXc"
geterror()         -> "Invalid subimage 0 requested (of -647316976 subimages)"

To Reproduce

Using a real damaged image from OpenEXR's own corpus. Any of the files in
openexr-images/Damaged/ that fails to open will do, and 11 of the first 14
do; this one fails with EXR_ERR_MISSING_REQ_ATTR 'channels' attribute not found. They are stored without a .exr extension, hence the copy.

mkdir udim && cd udim
cp .../openexr-images/Damaged/asan_heap-oob_504235_131_78b0e15388401193e1ee2ce20fb7f3b0_exr \
   tex.1001.exr

#include <OpenImageIO/imagecache.h>
#include <cstdio>

int main()
{
    auto ic = OIIO::ImageCache::create(false);
    int n           = -12345;
    const char* fmt = "UNTOUCHED";

    bool a = ic->get_image_info(OIIO::ustring("tex.<UDIM>.exr"), 0, 0,
                                OIIO::ustring("subimages"), OIIO::TypeInt, &n);
    bool b = ic->get_image_info(OIIO::ustring("tex.<UDIM>.exr"), 0, 0,
                                OIIO::ustring("fileformat"), OIIO::TypeString,
                                &fmt);
    std::printf("subimages  -> %s, wrote %d\n", a ? "true" : "false", n);
    std::printf("fileformat -> %s, pointer %p, string \"%s\"\n",
                b ? "true" : "false", (const void*)fmt,
                fmt ? fmt : "(null)");
    std::printf("geterror   -> \"%s\"\n", ic->geterror().c_str());
    return 0;
}

Expected: both return false, n stays -12345, fmt still reads
UNTOUCHED, and the reason is in geterror(). That is what the same file
gives when queried by its concrete name.

Observed, three fresh runs on main at 3e8576595:

subimages  -> true, wrote -659911935    fileformat -> true, pointer 00000095C96FFD80
subimages  -> true, wrote -492061951    fileformat -> true, pointer 00000030252FF650
subimages  -> true, wrote 427505921     fileformat -> true, pointer 000000BAE336FD20

Both calls report success, the int sentinel is replaced by a different value
each run, and the valid const char* is replaced by an address the caller is
expected to dereference. Linking needs OpenImageIO and OpenImageIO_Util.

The fix

Return false when no tile supplied a value, rather than copying a buffer that
was never written:

        if (first)
            return false;   // no readable tile had the attribute
        memcpy(data, common, size);
        return true;

first is already the exact condition, so this is one line.

A fuller answer would also report the reason. The two paths handle errors in
opposite directions and neither suits the skipped-tile case. The inner failure
path at :3138 clears the cache's whole error string before returning false,
and the skipped-tile path at :3128 clears nothing, so the call returns true
with the tile's EXR error still queued. That is visible in the evidence above:
geterror() holds EXR_ERR_FILE_BAD_HEADER after a call that returned
success. Reporting the first skipped tile's error on the all-skipped path would
turn a silent wrong answer into a diagnosable one.

Two in-tree consumers use the result without checking it. Both are reached
on ordinary paths, which is why this is worth more than a wrong return value.

ImageBuf::init_spec interns the copied-out pointer before validating
anything, src/libOpenImageIO/imagebuf.cpp:1176:

    m_fileformat = ustring(fmt);

The three queries above it at :1161-1167 all return true here, so the error
branch is skipped and fmt arrives holding the uninitialised bytes.
ustring(const char*) is null-safe but not garbage-safe,
src/include/OpenImageIO/ustring.h:140-143:

    explicit ustring(const char* str)
    {
        m_chars = str ? make_unique(str) : nullptr;
    }

so a non-null wild pointer goes to make_unique, which means strlen on an
arbitrary address. A zero would have been safe. An address is not.

testtex discards the return value outright and compares the pointer,
src/testtex/testtex.cpp:1962-1966:

    const char* texturetype = "Plain Texture";
    texsys->get_texture_info(filename, 0, ustring("texturetype"),
                             TypeDesc::STRING, &texturetype);
    Timer timer;
    if (!strcmp(texturetype, "Plain Texture")) {

No flag is needed to reach that, and on an all-unreadable set it is a strcmp
against uninitialised stack.

Test coverage

testsuite/texture-udim builds its tiles with oiiotool -otex and queries
them with testtex --gettextureinfo, so every tile is always readable. A
regression test would add one unreadable file at a UDIM index and assert that
the aggregate query returns false rather than true.