tiff: add thread-safe warning/error handlers, requires libtiff 4.5.0+ #4313
Conversation
Hmm, I was still able to reproduce that fuzzer bug. $ git diff projects/libvips/Dockerfile
diff --git a/projects/libvips/Dockerfile b/projects/libvips/Dockerfile
index 91787f1c8..10ef4abf5 100644
--- a/projects/libvips/Dockerfile
+++ b/projects/libvips/Dockerfile
@@ -35,7 +35,7 @@ RUN mkdir afl-testcases
RUN curl https://lcamtuf.coredump.cx/afl/demo/afl_testcases.tgz | tar xzC afl-testcases
RUN mkdir pdfium-latest
RUN curl -L https://github.com/bblanchon/pdfium-binaries/releases/latest/download/pdfium-linux-x64.tgz | tar xzC pdfium-latest
-RUN git clone --depth 1 https://github.com/libvips/libvips.git
+RUN git clone --depth 1 --branch tiffload-open-options https://github.com/lovell/libvips.git
RUN git clone --depth 1 https://github.com/madler/zlib.git
RUN git clone --depth 1 https://github.com/libexif/libexif.git
RUN git clone --depth 1 https://github.com/mm2/Little-CMS.git lcms
$ python infra/helper.py build_fuzzers --sanitizer address libvips > build_log.txt
$ python infra/helper.py reproduce libvips pngsave_buffer_fuzzer ~/Downloads/clusterfuzz-testcase-minimized-pngsave_buffer_fuzzer-4829207959830528
...
==9== ERROR: libFuzzer: out-of-memory (used: 2574Mb; limit: 2560Mb)
To change the out-of-memory limit use -rss_limit_mb=<N>
... |
|
I'm juggling quite a few branches at the moment and I must have tested that OSS Fuzz issue with the wrong code (or the wrong image), sorry Kleis. I did wonder why it hadn't auto-closed. I'll add support for the |
|
This is great Lovell! libtiff's global error handler has been a weeping sore for decades. It feels like a large change for a stable branch, and since everyone has been living with this for 30+ years, not time-critical either. My waters tell me this should be in 8.17. Is there an urgent issue this resolves? |
8927094 to
c87515f
Compare
|
I've updated the target branch to master for inclusion in the 8.17 release. |
|
Great! And let's aim for 8.17 six months after 8.16. |
Adds a compile-time test for libtiff
OpenOptionsfeature, when present sets the use of new, re-entrant error and warning handlers.Defining these (and having them
return 1) prevents the default behaviour of libtiff to call global, not thread-safe handlers shared between all users.The handlers also support user data, so
tiffloadcan now start to respect thefail_onoption. Homebrew currently provides libtiff 4.7.0 so we can test for this on macOS.I also investigated the various new memory limit safety features of
OpenOptionsdiscussed in #3892 but these only cover libtiff allocations e.g. parsing headers, and exclude allocations in its dependencies. Newer versions of libtiff already introduce more sanity checks around these anyway so the example from https://issues.oss-fuzz.com/issues/42531230 no longer fails.This PR targets the 8.16 branch as it fixes a long-standing potential thread-safety issue with libtiff. I guess this might be considered a breaking change only if someone is currently settingfail_on=warningbut expecting it not to 10000 .