search

AcademySoftwareFoundation / OpenImageIO

Reading, writing, and processing images in a wide variety of file formats, using a format-agnostic API, aimed at VFX applications.
https://openimageio.readthedocs.org
Apache License 2.0
1.98k stars 597 forks source link

[BUG] PNM reader uses variable before initialization, leading to undefined behaviour. #4445

Closed stolk closed 1 month ago

stolk commented 1 month ago

Describe the bug

Undefined behaviour by using unintialized value.

OpenImageIO version and dependencies

$ ./oiiotool --buildinfo
OIIO 2.6.7.0dev | Linux/x86_64
    Build compiler: clang 18.1 | C++17/201703
    HW features enabled at build: sse2
    No CUDA support (disabled / unavailable at build time)
Dependencies: BZip2 1.0.8, DCMTK NONE, FFmpeg NONE, fmt 10.2.1, Freetype 2.13.2, GIF 5.2.2, Imath 3.1.9, JPEG 80, JXL
    0.12.0, Libheif NONE, libjpeg-turbo NONE, LibRaw NONE, OpenColorIO 2.4.0, OpenCV NONE, OpenEXR 3.1.5, OpenGL, OpenJPEG
    NONE, OpenVDB NONE, PNG 1.6.43, Ptex NONE, Ptex NONE, pybind11 2.11.1, Python3 3.12.3, Qt5 NONE, Qt6 NONE, Robinmap
    1.3.0, TBB 2021.11.0, TIFF 4.5.1, WebP 1.3.2, ZLIB 1.3

To Reproduce

Run valgrind on imageinout_test binary.

Evidence

The reading of the value:

    Reading Proxy pnm ... ==75234== Conditional jump or move depends on uninitialised value(s)
==75234==    at 0x51A0EFA: OpenImageIO_v2_6_7::PNMInput::read_file_scanline(void*, int) (src/pnm.imageio/pnminput.cpp:210)
==75234==    by 0x51A1D78: OpenImageIO_v2_6_7::PNMInput::read_native_scanline(int, int, int, int, void*) (src/pnm.imageio/pnminput.cpp:404)
==75234==    by 0x4FBDA81: OpenImageIO_v2_6_7::ImageInput::read_native_scanlines(int, int, int, int, int, void*) (src/libOpenImageIO/imageinput.cpp:417)
==75234==    by 0x4FBDBDA: OpenImageIO_v2_6_7::ImageInput::read_native_scanlines(int, int, int, int, int, int, int, void*) (src/libOpenImageIO/imageinput.cpp:438)
==75234==    by 0x4FBD35B: OpenImageIO_v2_6_7::ImageInput::read_scanlines(int, int, int, int, int, int, int, OpenImageIO_v2_6_7::TypeDesc, void*, long, long) (src/libOpenImageIO/imageinput.cpp:354)
==75234==    by 0x4FC12A2: OpenImageIO_v2_6_7::ImageInput::read_image(int, int, int, int, OpenImageIO_v2_6_7::TypeDesc, void*, long, long, long, bool (*)(void*, float), void*) (src/libOpenImageIO/imageinput.cpp:992)
==75234==    by 0x119C98: checked_read(OpenImageIO_v2_6_7::ImageInput*, OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, std::vector<unsigned char, std::allocator<unsigned char> >&, bool, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) (src/libOpenImageIO/imageinout_test.cpp:138)
==75234==    by 0x11BAB0: test_read_proxy(OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, OpenImageIO_v2_6_7::ImageBuf const&) (src/libOpenImageIO/imageinout_test.cpp:277)
==75234==    by 0x11818E: test_all_formats() (src/libOpenImageIO/imageinout_test.cpp:423)
==75234==    by 0x116D76: main (src/libOpenImageIO/imageinout_test.cpp:532)

The creation of the value:

==75234==  Uninitialised value was created by a heap allocation
==75234==    at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==75234==    by 0x4FBB754: OpenImageIO_v2_6_7::ImageInput::operator new(unsigned long) (src/libOpenImageIO/imageinput.cpp:66)
==75234==    by 0x51A0DD1: OpenImageIO_v2_6_7::pnm_input_imageio_create() (src/pnm.imageio/pnminput.cpp:90)
==75234==    by 0x4FE373F: OpenImageIO_v2_6_7::ImageInput::create(OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, bool, OpenImageIO_v2_6_7::ImageSpec const*, OpenImageIO_v2_6_7::Filesystem::IOProxy*, OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >) (src/libOpenImageIO/imageioplugin.cpp:664)
==75234==    by 0x4FBBF72: OpenImageIO_v2_6_7::ImageInput::open(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, OpenImageIO_v2_6_7::ImageSpec const*, OpenImageIO_v2_6_7::Filesystem::IOProxy*) (src/libOpenImageIO/imageinput.cpp:154)
==75234==    by 0x11B7F8: test_read_proxy(OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, OpenImageIO_v2_6_7::basic_string_view<char, std::char_traits<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, OpenImageIO_v2_6_7::ImageBuf const&) (src/libOpenImageIO/imageinout_test.cpp:273)
==75234==    by 0x11818E: test_all_formats() (src/libOpenImageIO/imageinout_test.cpp:423)
==75234==    by 0x116D76: main (src/libOpenImageIO/imageinout_test.cpp:532)
==75234==
stolk commented 1 month ago

This is due to not always setting m_pfm_flip before being used.

I will make an PR to address this.

stolk commented 1 month ago

https://github.com/AcademySoftwareFoundation/OpenImageIO/pull/4446