Skip to content

Commit 1dff182

Browse files
authored
ci: Fix UB in exif parse, revert PR #5113, move sanitizer test to 2026 (#5120)
PR #5113 didn't fix all the problems after all. Revert it, because it's not needed after the real solution. The real solution is to mark tiff_datatype_to_typedesc() with OIIO_NO_SANITIZE_UNDEFINED to have the sanitizer skip it. What's happening is that corrupted files end up with a value in the data type field that is not a valid value of the TIFFDataType enum, and ubsan is flagging that. BUT... `tiff_datatype_to_typedesc()` itself is actually safe in that circumstance, because it's got a 'default` clause that handles the unknown enum values just fine. In contrast, I've been running around in circles trying to find something to do *within* that function to make it "safe" (by the sanitizer's reckoning), and trying to check for valid values prior to converting to a TIFFDataType is just too hard, partly because there are places where that conversion happens unchecked inside libtiff (C language, just does a cast), so it comes to us as a TIFFDataType already with an invalid value. It's easier to just mark the whole function as being ignored by ubsan, given that we can see by inspection that it has totally deterministic and desired behavior for the illegal values. Making this spurious ubsan error disappear allows us to upgrade the container version we are using for the "sanitizer" CI test variant, so we also push that all the way up to 2026. Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 48181b2 commit 1dff182

File tree

3 files changed

+11
-20
lines changed

3 files changed

+11
-20
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,11 @@ jobs:
347347
- desc: Sanitizers
348348
nametag: sanitizer
349349
runner: ubuntu-latest
350-
container: aswf/ci-oiio:2024.2
350+
container: aswf/ci-oiio:2026.3
351351
cc_compiler: clang
352352
cxx_compiler: clang++
353353
build_type: Debug
354-
opencolorio_ver: v2.4.2
355-
python_ver: "3.11"
354+
python_ver: "3.13"
356355
ctest_test_timeout: "1200"
357356
setenvs: export SANITIZE=address,undefined
358357
OIIO_CMAKE_FLAGS="-DSANITIZE=address,undefined -DOIIO_HARDENING=3 -DUSE_PYTHON=0"

src/include/OpenImageIO/tiffutils.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,9 @@ OIIO_NAMESPACE_3_1_BEGIN
159159
/// obvious equivalent.
160160
OIIO_API TypeDesc tiff_datatype_to_typedesc (TIFFDataType tifftype, size_t tiffcount=1);
161161

162-
OIIO_API TypeDesc tiff_datatype_to_typedesc (const TIFFDirEntry& dir);
162+
inline TypeDesc tiff_datatype_to_typedesc (const TIFFDirEntry& dir) {
163+
return tiff_datatype_to_typedesc (TIFFDataType(dir.tdir_type), dir.tdir_count);
164+
}
163165

164166
/// Return the data size (in bytes) of the TIFF type.
165167
OIIO_API size_t tiff_data_size (TIFFDataType tifftype);

src/libOpenImageIO/exif.cpp

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -178,22 +178,12 @@ tiff_data_size(const TIFFDirEntry& dir)
178178

179179

180180

181-
TypeDesc
182-
tiff_datatype_to_typedesc(const TIFFDirEntry& dir)
183-
{
184-
// Check for corrupt/invalid value
185-
#if defined(TIFF_VERSION_BIG)
186-
if (dir.tdir_type > TIFF_IFD8)
187-
#else
188-
if (dir.tdir_type > TIFF_IFD)
189-
#endif
190-
return TypeUnknown;
191-
return tiff_datatype_to_typedesc(TIFFDataType(dir.tdir_type),
192-
dir.tdir_count);
193-
}
194-
195-
196-
181+
// N.B. We have to turn undefined behavior sanitizer off for this function
182+
// because ubsan will complain about tifftype being an invalid enum value in
183+
// cases where the value really is corrupted and invalid -- but the switch
184+
// statement here is nonetheless safe because it just returns TypeUnknown in
185+
// such cases.
186+
OIIO_NO_SANITIZE_UNDEFINED
197187
TypeDesc
198188
tiff_datatype_to_typedesc(TIFFDataType tifftype, size_t tiffcount)
199189
{

0 commit comments

Comments
 (0)