Refactor ImfCheckFile and oss-fuzz tests (#1257)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
AcademySoftwareFoundation · May 15, 2022 · 672c77d · 672c77d
1 parent 3d0a184
commit 672c77d
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 57 deletions.
diff --git a/src/lib/OpenEXR/ImfHeader.cpp b/src/lib/OpenEXR/ImfHeader.cpp
@@ -1270,6 +1270,21 @@ Header::setMaxTileSize (int maxWidth, int maxHeight)
     maxTileHeight = maxHeight;
 }
 
+void
+Header::getMaxImageSize (int& maxWidth, int& maxHeight)
+{
+    maxWidth = maxImageWidth;
+    maxHeight = maxImageHeight;
+}
+
+void
+Header::getMaxTileSize (int& maxWidth, int& maxHeight)
+{
+    maxWidth = maxTileWidth;
+    maxHeight= maxTileHeight;
+}
+
+
 bool
 Header::readsNothing ()
 {

diff --git a/src/lib/OpenEXR/ImfHeader.h b/src/lib/OpenEXR/ImfHeader.h
@@ -446,6 +446,11 @@ class IMF_EXPORT_TYPE Header
     static void setMaxImageSize (int maxWidth, int maxHeight);
     IMF_EXPORT
     static void setMaxTileSize (int maxWidth, int maxHeight);
+    IMF_EXPORT
+    static void getMaxImageSize (int& maxWidth, int& maxHeight);
+    IMF_EXPORT
+    static void getMaxTileSize (int& maxWidth, int& maxHeight);
+
 
     //
     // Check if the header reads nothing.

diff --git a/src/lib/OpenEXRUtil/ImfCheckFile.cpp b/src/lib/OpenEXRUtil/ImfCheckFile.cpp
@@ -565,11 +565,6 @@ readDeepTile (T& in, bool reduceMemory, bool reduceTime)
 
         Box2i dataWindow = fileHeader.dataWindow ();
 
-        //
-        // use uint64_t for dimensions, since dataWindow+1 could overflow int storage
-        //
-        uint64_t height = static_cast<uint64_t> (dataWindow.size ().y) + 1;
-        uint64_t width  = static_cast<uint64_t> (dataWindow.size ().x) + 1;
         int      bytesPerSample = calculateBytesPerPixel (in.header ());
 
         const TileDescription& td         = in.header ().tileDescription ();
@@ -1030,6 +1025,30 @@ template <class T>
 bool
 runChecks (T& source, bool reduceMemory, bool reduceTime)
 {
+
+    //
+    // in reduceMemory/reduceTime mode, limit image size, tile size, and maximum deep samples
+    //
+
+    uint64_t oldMaxSampleCount = CompositeDeepScanLine::getMaximumSampleCount();
+
+    int maxImageWidth , maxImageHeight;
+    Header::getMaxImageSize(maxImageWidth,maxImageHeight);
+
+    int maxTileWidth , maxTileHeight;
+    Header::getMaxImageSize(maxTileWidth,maxTileHeight);
+
+
+    if( reduceMemory || reduceTime)
+    {
+        CompositeDeepScanLine::setMaximumSampleCount(1<<20);
+        Header::setMaxImageSize(2048,2048);
+        Header::setMaxTileSize(512,512);
+    }
+
+
+
+
     //
     // multipart test: also grab the type of the first part to
     // check which other tests are expected to fail
@@ -1048,7 +1067,6 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
     // If the MultiPartInputFile constructor throws an exception, the first part
     // will assumed to be a wide image
     //
-    bool firstPartWide = true;
     bool largeTiles    = true;
 
     bool threw = false;
@@ -1057,17 +1075,6 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
         {
             MultiPartInputFile multi (source);
             Box2i              b          = multi.header (0).dataWindow ();
-            uint64_t           imageWidth = static_cast<uint64_t> (b.max.x) -
-                                  static_cast<uint64_t> (b.min.x) + 1ll;
-            uint64_t bytesPerPixel = calculateBytesPerPixel (multi.header (0));
-            uint64_t numLines =
-                numLinesInBuffer (multi.header (0).compression ());
-
-            // confirm first part is small enough to read without using excessive memory
-            if (imageWidth * bytesPerPixel * numLines <= gMaxBytesPerScanline)
-            {
-                firstPartWide = false;
-            }
 
             //
             // significant memory is also required to read a tiled file
@@ -1080,18 +1087,10 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
             {
                 const TileDescription& tileDescription =
                     multi.header (0).tileDescription ();
-                uint64_t tilesPerScanline =
-                    (imageWidth + tileDescription.xSize - 1ll) /
-                    tileDescription.xSize;
                 uint64_t tileSize =
                     static_cast<uint64_t> (tileDescription.xSize) *
                     static_cast<uint64_t> (tileDescription.ySize);
                 int bytesPerPixel = calculateBytesPerPixel (multi.header (0));
-                if (tileSize * tilesPerScanline * bytesPerPixel >
-                    gMaxTileBytesPerScanline)
-                {
-                    firstPartWide = true;
-                }
 
                 if (tileSize * bytesPerPixel <= gMaxTileBytes)
                 {
@@ -1115,7 +1114,7 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
     }
 
     // read using both scanline interfaces (unless the image is wide and reduce memory enabled)
-    if (!reduceMemory || !firstPartWide)
+    if (!reduceMemory)
     {
         {
             bool gotThrow = false;
@@ -1163,7 +1162,7 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
         if (gotThrow && firstPartType == TILEDIMAGE) { threw = true; }
     }
 
-    if (!reduceMemory || !firstPartWide)
+    if (!reduceMemory)
     {
         bool gotThrow = false;
         resetInput (source);
@@ -1195,6 +1194,12 @@ runChecks (T& source, bool reduceMemory, bool reduceTime)
         if (gotThrow && firstPartType == DEEPTILE) { threw = true; }
     }
 
+
+
+    CompositeDeepScanLine::setMaximumSampleCount(oldMaxSampleCount);
+    Header::setMaxImageSize(maxImageWidth,maxImageHeight);
+    Header::setMaxTileSize(maxTileWidth,maxTileHeight);
+
     return threw;
 }
 
@@ -1621,26 +1626,18 @@ checkOpenEXRFile (
     const char* fileName,
     bool        reduceMemory,
     bool        reduceTime,
-    bool        enableCoreCheck)
+    bool        runCoreCheck)
 {
-    bool threw = false;
-
-    uint64_t oldMaxSampleCount = CompositeDeepScanLine::getMaximumSampleCount();
 
-    if( reduceMemory || reduceTime)
+    if (runCoreCheck)
     {
-        CompositeDeepScanLine::setMaximumSampleCount(1<<20);
+        return runCoreChecks (fileName, reduceMemory, reduceTime);
     }
-
-    if (enableCoreCheck)
+    else
     {
-        threw = runCoreChecks (fileName, reduceMemory, reduceTime);
+        return runChecks (fileName, reduceMemory, reduceTime);
     }
-    if (!threw) threw = runChecks (fileName, reduceMemory, reduceTime);
 
-    CompositeDeepScanLine::setMaximumSampleCount(oldMaxSampleCount);
-
-    return threw;
 }
 
 bool
@@ -1649,27 +1646,21 @@ checkOpenEXRFile (
     size_t      numBytes,
     bool        reduceMemory,
     bool        reduceTime,
-    bool        enableCoreCheck)
+    bool        runCoreCheck)
 {
-    bool threw = false;
-    uint64_t oldMaxSampleCount = CompositeDeepScanLine::getMaximumSampleCount();
 
-    if( reduceMemory || reduceTime)
-    {
-        CompositeDeepScanLine::setMaximumSampleCount(1<<20);
-    }
 
-    if (enableCoreCheck)
-        threw = runCoreChecks (data, numBytes, reduceMemory, reduceTime);
-    if (!threw)
-    {
+     if (runCoreCheck)
+     {
+        return runCoreChecks (data, numBytes, reduceMemory, reduceTime);
+     }
+     else
+     {
         PtrIStream stream (data, numBytes);
-        threw = runChecks (stream, reduceMemory, reduceTime);
+        return runChecks (stream, reduceMemory, reduceTime);
     }
 
-    CompositeDeepScanLine::setMaximumSampleCount(oldMaxSampleCount);
 
-    return threw;
 }
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_SOURCE_EXIT
diff --git a/src/lib/OpenEXRUtil/ImfCheckFile.h b/src/lib/OpenEXRUtil/ImfCheckFile.h
@@ -25,13 +25,14 @@ OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 // if reduceTime is true and an error is found within the file, then future tests are reduced for speed.
 // This may hide errors within the library.
 //
+// if runCoreCheck is true, only uses the OpenEXRCore (C) API, otherwise uses the OpenEXR (C++) API
 //
 
 IMFUTIL_EXPORT bool checkOpenEXRFile (
     const char* fileName,
     bool        reduceMemory    = false,
     bool        reduceTime      = false,
-    bool        enableCoreCheck = false);
+    bool        runCoreCheck = false);
 
 //
 // overloaded version of checkOpenEXRFile that takes a pointer to in-memory data
@@ -42,7 +43,7 @@ IMFUTIL_EXPORT bool checkOpenEXRFile (
     size_t      numBytes,
     bool        reduceMemory    = false,
     bool        reduceTime      = false,
-    bool        enableCoreCheck = false);
+    bool        runCoreCheck = false);
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT
 

diff --git a/src/test/OpenEXRFuzzTest/oss-fuzz/openexr_exrcheck_fuzzer.cc b/src/test/OpenEXRFuzzTest/oss-fuzz/openexr_exrcheck_fuzzer.cc
@@ -14,7 +14,7 @@
 using OPENEXR_IMF_NAMESPACE::checkOpenEXRFile;
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 {
-    checkOpenEXRFile ((const char*) data , size , true , true, true);
+    checkOpenEXRFile ((const char*) data , size , true , true, false);
     return 0;
 }
 
diff --git a/src/test/OpenEXRFuzzTest/oss-fuzz/openexr_exrcorecheck_fuzzer.cc b/src/test/OpenEXRFuzzTest/oss-fuzz/openexr_exrcorecheck_fuzzer.cc
@@ -0,0 +1,20 @@
+
+//
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) Contributors to the OpenEXR Project.
+//
+// this file is found by the oss-fuzz project to generate a fuzzer. It is not part of
+// OpenEXR's internal OpenEXRFuzzTest suite
+//
+
+#include <ImfNamespace.h>
+#include <ImfCheckFile.h>
+#include <stdint.h>
+
+using OPENEXR_IMF_NAMESPACE::checkOpenEXRFile;
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+    checkOpenEXRFile ((const char*) data , size , true , true, true);
+    return 0;
+}
+