Skip to content

Commit 8835db0

Browse files
committed
test/oss-fuzz: address Copilot review on openexr_exrgaps_fuzzer
Six fixes from the Copilot review of #2401: 1. read_u32(): cast each byte to uint32_t before shifting/ORing. The previous form left-shifted promoted-to-int values, which is undefined behavior in C++ when the shifted value would set the sign bit. 2. write_temp(): handle short writes and EINTR. The single write() call could legally return early or be interrupted, causing valid inputs to be silently dropped. Loop until the full buffer is written, retrying on EINTR. 3. Add explicit #include <algorithm> for std::min, which was previously relying on transitive inclusion. Also add <cerrno>. 4. Mode 0 (test_tiled_random_coords): replace `if (hdr.find("type") != hdr.end()) continue;` with a check on the actual type value. MultiPartInputFile auto-synthesizes a "type" attribute when missing, so the previous check effectively skipped every part. Mirror the fix in test_deep_tiled to require hdr.type() == DEEPTILE explicitly. Use Imf::DEEPTILE constants from ImfPartType.h. 5. Mode 1 (test_tiled_range): clamp the (dx2-dx1+1)*(dy2-dy1+1) product so a single readTiles() call requests at most kMaxRangeSide^2 = 64 tiles, down from the prior 32x32 = 1024. Improves fuzz throughput on valid tiled inputs without losing coverage of small-range and boundary cases. 6. CMakeLists.txt: replace the duplicated fuzzer list in the oss_fuzz custom target's DEPENDS with ${OPENEXR_FUZZERS}, so the list is maintained in one place. Signed-off-by: Anthony Hurtado <amhurtado@pm.me>
1 parent 52a6cec commit 8835db0

2 files changed

Lines changed: 40 additions & 14 deletions

File tree

src/test/oss-fuzz/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ message(status "CC_FLAGS=$ENV{CC_FLAGS}")
2626

2727
set(OPENEXR_FUZZERS openexr_exrcheck_fuzzer openexr_exrcorecheck_fuzzer openexr_encoding_fuzzer openexr_attribute_fuzzer openexr_htj2k_fuzzer openexr_roundtrip_fuzzer openexr_exrgaps_fuzzer)
2828
add_custom_target(oss_fuzz ALL
29-
DEPENDS openexr_exrcheck_fuzzer openexr_exrcorecheck_fuzzer openexr_encoding_fuzzer openexr_attribute_fuzzer openexr_htj2k_fuzzer openexr_roundtrip_fuzzer openexr_exrgaps_fuzzer
29+
DEPENDS ${OPENEXR_FUZZERS}
3030
)
3131

3232
if ($ENV{FUZZING_ENGINE} STREQUAL "afl")

src/test/oss-fuzz/openexr_exrgaps_fuzzer.cc

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
// C++ MultiPartInputFile/TiledInputPart API operates on file paths.
2727
//
2828

29+
#include <algorithm>
30+
#include <cerrno>
2931
#include <cstdint>
3032
#include <cstddef>
3133
#include <cstdio>
@@ -42,23 +44,41 @@
4244
#include <ImfFrameBuffer.h>
4345
#include <ImfDeepFrameBuffer.h>
4446
#include <ImfTileDescriptionAttribute.h>
47+
#include <ImfPartType.h>
4548

4649
namespace IMF = OPENEXR_IMF_NAMESPACE;
4750

4851
namespace {
4952

50-
constexpr size_t kMaxInput = 4 * 1024 * 1024;
51-
constexpr int kMaxTilesPerCall = 16;
52-
constexpr int kMaxParts = 8;
53+
constexpr size_t kMaxInput = 4 * 1024 * 1024;
54+
constexpr int kMaxTilesPerCall = 16;
55+
constexpr int kMaxParts = 8;
56+
// Bound on (dx2-dx1+1)*(dy2-dy1+1) for readTiles() to avoid pathological
57+
// 1024-tile calls slowing fuzz throughput. 8x8 = 64 tiles per call covers
58+
// boundary and small-range cases without burning iteration time.
59+
constexpr int kMaxRangeSide = 8;
5360

5461
std::string write_temp (const uint8_t* data, size_t size)
5562
{
5663
char path[] = "/tmp/exr_gaps_XXXXXX";
5764
int fd = mkstemp (path);
5865
if (fd < 0) return std::string ();
59-
ssize_t n = write (fd, data, size);
66+
size_t total = 0;
67+
while (total < size)
68+
{
69+
ssize_t n = write (fd, data + total, size - total);
70+
if (n < 0)
71+
{
72+
if (errno == EINTR) continue;
73+
close (fd);
74+
unlink (path);
75+
return std::string ();
76+
}
77+
if (n == 0) break;
78+
total += static_cast<size_t> (n);
79+
}
6080
close (fd);
61-
if (n != static_cast<ssize_t> (size))
81+
if (total != size)
6282
{
6383
unlink (path);
6484
return std::string ();
@@ -69,7 +89,10 @@ std::string write_temp (const uint8_t* data, size_t size)
6989
uint32_t read_u32 (const uint8_t* p, size_t size, size_t off)
7090
{
7191
if (off + 4 > size) return 0;
72-
return p[off] | (p[off + 1] << 8) | (p[off + 2] << 16) | (p[off + 3] << 24);
92+
return static_cast<uint32_t> (p[off])
93+
| (static_cast<uint32_t> (p[off + 1]) << 8)
94+
| (static_cast<uint32_t> (p[off + 2]) << 16)
95+
| (static_cast<uint32_t> (p[off + 3]) << 24);
7396
}
7497

7598
void
@@ -85,8 +108,10 @@ test_tiled_random_coords (
85108
{
86109
const IMF::Header& hdr = mpif.header (p);
87110
if (!hdr.hasTileDescription ()) continue;
88-
auto type_it = hdr.find ("type");
89-
if (type_it != hdr.end ()) continue;
111+
// TiledInputPart is for non-deep tiled parts. Mode 2 covers
112+
// deep tiled. MultiPartInputFile auto-synthesizes a "type"
113+
// attribute, so we must check the value rather than presence.
114+
if (hdr.hasType () && hdr.type () == IMF::DEEPTILE) continue;
90115
try
91116
{
92117
IMF::TiledInputPart tip (mpif, p);
@@ -142,9 +167,10 @@ test_tiled_range (
142167
uint32_t r = read_u32 (params, param_size, 0);
143168
uint32_t s = read_u32 (params, param_size, 4);
144169
int dx1 = r & 0xff;
145-
int dx2 = dx1 + ((r >> 8) & 0x1f);
146170
int dy1 = (r >> 16) & 0xff;
147-
int dy2 = dy1 + ((r >> 24) & 0x1f);
171+
// Clamp range size so total tile count stays <= kMaxRangeSide^2.
172+
int dx2 = dx1 + (((r >> 8) & 0x1f) % kMaxRangeSide);
173+
int dy2 = dy1 + (((r >> 24) & 0x1f) % kMaxRangeSide);
148174
int lx = s & 0x0f;
149175
int ly = (s >> 4) & 0x0f;
150176
try
@@ -176,9 +202,9 @@ test_deep_tiled (
176202
int n_parts = std::min (mpif.parts (), kMaxParts);
177203
for (int p = 0; p < n_parts; p++)
178204
{
179-
const IMF::Header& hdr = mpif.header (p);
180-
auto type_it = hdr.find ("type");
181-
if (type_it == hdr.end ()) continue;
205+
const IMF::Header& hdr = mpif.header (p);
206+
// DeepTiledInputPart only accepts deep tiled parts.
207+
if (!hdr.hasType () || hdr.type () != IMF::DEEPTILE) continue;
182208
try
183209
{
184210
IMF::DeepTiledInputPart dtip (mpif, p);

0 commit comments

Comments
 (0)