Invalid shift (141647077)

[Google-Autofuzz]

Hello OpenEXR team,

As part of our fuzzing efforts at Google, we have identified an issue affecting
OpenEXR (tested with revision * master e23fdf6).

To reproduce, we are attaching a Dockerfile which compiles the project with
LLVM, taking advantage of the sanitizers that it offers. More information about
how to use the attached Dockerfile can be found here:
https://docs.docker.com/engine/reference/builder/

Instructions:
unzip artifacts_141647077.zip
docker build --build-arg SANITIZER=undefined --tag=autofuzz-OpenEXR-141647077 autofuzz_141647077
docker run --entrypoint /fuzzing/repro.sh --cap-add=SYS_PTRACE -v $PWD/autofuzz_141647077/autofuzz_141647077:/tmp/poc autofuzz-OpenEXR-141647077 "exrenvmap" /tmp/poc
docker run --cap-add=SYS_PTRACE -v $PWD/autofuzz_141647077/autofuzz_141647077:/tmp/poc -it autofuzz-OpenEXR-141647077

Alternatively, and depending on the bug, you could use gcc, valgrind or other
instrumentation tools to aid in the investigation. The sanitizer error that we
encountered is here:

/fuzzing/openexr/OpenEXR/IlmImf/ImfXdr.h:679:9: runtime error: left shift of negative value -79
    #0 0x7fb3547ed74a in void Imf_2_4::Xdr::read<Imf_2_4::StreamIO, Imf_2_4::IStream>(Imf_2_4::IStream&, int&) /fuzzing/openexr/OpenEXR/IlmImf/ImfXdr.h:679:9
    #1 0x7fb3548ddb8f in Imf_2_4::(anonymous namespace)::readPixelData(Imf_2_4::InputStreamMutex*, Imf_2_4::ScanLineInputFile::Data*, int, char*&, int&) /fuzzing/openexr/OpenEXR/IlmImf/ImfScanLineInputFile.cpp:436:5
    #2 0x7fb3548dc7ff in Imf_2_4::(anonymous namespace)::newLineBufferTask(IlmThread_2_4::TaskGroup*, Imf_2_4::InputStreamMutex*, Imf_2_4::ScanLineInputFile::Data*, int, int, int, Imf_2_4::OptimizationMode) /fuzzing/openexr/OpenEXR/IlmImf/ImfScanLineInputFile.cpp:1044:14
    #3 0x7fb3548dba0d in Imf_2_4::ScanLineInputFile::readPixels(int, int) /fuzzing/openexr/OpenEXR/IlmImf/ImfScanLineInputFile.cpp:1679:44
    #4 0x429455 in (anonymous namespace)::readSingleImage(char const*, float, float, Imf_2_4::Envmap, bool, EnvmapImage&, Imf_2_4::Header&, Imf_2_4::RgbaChannels&) /fuzzing/openexr/OpenEXR/exrenvmap/readInputImage.cpp:118:8
    #5 0x42cd8b in main /fuzzing/openexr/OpenEXR/exrenvmap/main.cpp:471:9
    #6 0x7fb353c1409a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #7 0x4068d9 in _start (/fuzzing/openexr/build/bin/exrenvmap+0x4068d9)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /fuzzing/openexr/OpenEXR/IlmImf/ImfXdr.h:679:9 in

We will gladly work with you so you can successfully confirm and reproduce this
issue. Do let us know if you have any feedback surrounding the documentation.

Once you have reproduced the issue, we'd appreciate to learn your expected
timeline for an update to be released. With any fix, please attribute the report
to "Google Autofuzz project".

We are also pleased to inform you that your project is eligible for inclusion to
the OSS-Fuzz project, which can provide additional continuous fuzzing, and
encourage you to investigate integration options.

Don't hesitate to let us know if you have any questions!

Google AutoFuzz Team

artifacts_141647077.zip

[peterhillman]

I've reproduced this.
Left shift of negative values are technically undefined, though in this case compilers will generally work correctly, due to the bitmasking that is applied directly afterwards.

Also, there is an exhaustive test of Xdr in IlmImfTest, so any unexpected compiler behavior will be detected there. It's possible to modify Xdr.h so that errors are not reported by the 'undefined behavior' sanitizer. However, the undefined behavior sanitizer running on IlmImfTest reports various other issues across the codebase. None of the ones I examined appear to be problematic, and we already cover them in our test suite.

I'm not keen to consider reports from the undefined behavior sanitizer as critical errors, unless they can be shown to cause more serious problems such as security issues or memory leaks. I would be concerned that modifying the code to circumvent sanitizer report messages may introduce subtle bugs or hamper the library's performance with no practical benefit.

[cary-ilm]

To add to what @peterhillman said, we're confident there is not a legitimate concern here, validated by the test suite, and we'd prefer not to complicate the code in order to simply remove the warning. Closing the issue for now. If you have a alternative resolution, we'd be happy to discuss it further.

[Google-Autofuzz]

If this is known to not be any kind of problem, one option to consider would be annotating the known safe functions with attributes to avoid sanitizer instrumentation:

https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-undefined

That can prevent these shallow crashes from triggering, and allow fuzzing of code paths beyond them, but obviously it's up to you if you want to include those annotations or not.

Thanks for taking a look at this issue!