Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CVE 2023 5841 #1627

Merged
merged 4 commits into from Feb 4, 2024
Merged

Conversation

kdt3rd
Copy link
Contributor

@kdt3rd kdt3rd commented Feb 3, 2024

the deep sample validation was only comparing against 0, not that it was monotonic, allowing malicious files to be constructed

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but I have some quibbles about types :)

src/lib/OpenEXRCore/decoding.c Show resolved Hide resolved
src/lib/OpenEXRCore/decoding.c Show resolved Hide resolved
src/lib/OpenEXRCore/decoding.c Show resolved Hide resolved
UNPACK_SAMPLES (samps)
}
srcbuffer += bpc * samps;
srcbuffer += ((size_t) bpc) * ((size_t) samps);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size_t is a suspicious type (one of those bonkers "haha could be anything" types). ptrdiff_t is probably the right choice here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i just used pre-existing type. size_t is definitely variable, ptrdiff_t would be fine

outc.user_pixel_stride = outc.user_bytes_per_element;
outc.user_line_stride = outc.user_pixel_stride * width;

dptr += width * (uint64_t) outc.user_bytes_per_element *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ptrdiff_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, was just avoiding int32 math problems

@limburgher
Copy link

I maintain the 2.x series of OpenEXR for Fedora (openexr2 package). The report I got indicate that this CVE impacts that as well but the codebase is entirely different to 3.x. Can someone with more experience with it confirm that whether it is indeed impacted or not?

@kdt3rd
Copy link
Contributor Author

kdt3rd commented Feb 3, 2024

I maintain the 2.x series of OpenEXR for Fedora (openexr2 package). The report I got indicate that this CVE impacts that as well but the codebase is entirely different to 3.x. Can someone with more experience with it confirm that whether it is indeed impacted or not?

The CVE over-inflates the impact of this, IMO. For one, this is new code that does not exist in openexr 2.x, and does not exist in early versions of 3.x - although has existed since 2021. Second, this is only used in the ImfCheckFile code of the C++ API currently, which is the API predominantly used currently. Additionally, checkFile is normally only used during development / testing / etc, and not something you would use in production code - you wouldn't test that you can read the entire file, reading the entire file, discard that data, to then just read the entire file again and use it.

It is of course a good bug to fix, and one reason why we haven't finished pushing the refactored code into the mainline C++, waiting for more fuzz / security issues to expose themselves. But this is not in 2.x, the C++ side has been correct since 2013, and ultimately the real fix was fixing a typo from if (samps < 0) to if (samps < prevsamps) in the code to ensure the file is monotonically increasing in the number of samples. The other changes here are additional checks and cleanup, and enabling this code to be checked more rapidly by the fuzz tool

Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cary-ilm cary-ilm merged commit 46944c3 into AcademySoftwareFoundation:main Feb 4, 2024
29 of 30 checks passed
cary-ilm pushed a commit that referenced this pull request Feb 4, 2024
* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
cary-ilm pushed a commit that referenced this pull request Feb 4, 2024
* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
@limburgher
Copy link

I maintain the 2.x series of OpenEXR for Fedora (openexr2 package). The report I got indicate that this CVE impacts that as well but the codebase is entirely different to 3.x. Can someone with more experience with it confirm that whether it is indeed impacted or not?

The CVE over-inflates the impact of this, IMO. For one, this is new code that does not exist in openexr 2.x, and does not exist in early versions of 3.x - although has existed since 2021. Second, this is only used in the ImfCheckFile code of the C++ API currently, which is the API predominantly used currently. Additionally, checkFile is normally only used during development / testing / etc, and not something you would use in production code - you wouldn't test that you can read the entire file, reading the entire file, discard that data, to then just read the entire file again and use it.

It is of course a good bug to fix, and one reason why we haven't finished pushing the refactored code into the mainline C++, waiting for more fuzz / security issues to expose themselves. But this is not in 2.x, the C++ side has been correct since 2013, and ultimately the real fix was fixing a typo from if (samps < 0) to if (samps < prevsamps) in the code to ensure the file is monotonically increasing in the number of samples. The other changes here are additional checks and cleanup, and enabling this code to be checked more rapidly by the fuzz tool

I suspected as much, thank you for confirming!

@kdt3rd kdt3rd deleted the fix_cve_2023_5841 branch February 6, 2024 01:03
cary-ilm pushed a commit to cary-ilm/openexr that referenced this pull request Feb 11, 2024
* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
cary-ilm added a commit that referenced this pull request Feb 13, 2024
* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix typo

Signed-off-by: Cary Phillips <cary@ilm.com>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add copyright notice

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <cary@ilm.com>

* test1

Signed-off-by: Cary Phillips <cary@ilm.com>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <cary@ilm.com>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Bazel support: Bump Imath to 3.1.10 (#1626)

Signed-off-by: Vertexwahn <julian.amann@tum.de>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <cary@ilm.com>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update SECURITY.md

Co-authored-by: Nick Porcino <meshula@hotmail.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <cary@ilm.com>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <cary@ilm.com>

* github security advisory

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention exrcheck

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Nick Porcino <meshula@hotmail.com>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Remove snyk-scan-pr.yml (#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix issue with unpacking sample counts (#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* adjust checks for core to better match c++ checks (#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Fix install of symlink (#1633)

PR #1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <cary@ilm.com>

* adds a shortcut to avoid reconstructing every call (#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* check and control reduceMemory and reduceTime in stream mode (#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in #1634

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add sdist

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Co-authored-by: Kimball Thurston <kdt3rd@gmail.com>
Co-authored-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Nick Porcino <meshula@hotmail.com>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
…1629)

* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix typo

Signed-off-by: Cary Phillips <cary@ilm.com>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add copyright notice

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <cary@ilm.com>

* test1

Signed-off-by: Cary Phillips <cary@ilm.com>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <cary@ilm.com>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Bazel support: Bump Imath to 3.1.10 (AcademySoftwareFoundation#1626)

Signed-off-by: Vertexwahn <julian.amann@tum.de>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <cary@ilm.com>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update SECURITY.md

Co-authored-by: Nick Porcino <meshula@hotmail.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <cary@ilm.com>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <cary@ilm.com>

* github security advisory

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention exrcheck

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Nick Porcino <meshula@hotmail.com>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Remove snyk-scan-pr.yml (AcademySoftwareFoundation#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix issue with unpacking sample counts (AcademySoftwareFoundation#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* adjust checks for core to better match c++ checks (AcademySoftwareFoundation#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Fix install of symlink (AcademySoftwareFoundation#1633)

PR AcademySoftwareFoundation#1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <cary@ilm.com>

* adds a shortcut to avoid reconstructing every call (AcademySoftwareFoundation#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* check and control reduceMemory and reduceTime in stream mode (AcademySoftwareFoundation#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in AcademySoftwareFoundation#1634

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add sdist

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Co-authored-by: Kimball Thurston <kdt3rd@gmail.com>
Co-authored-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Nick Porcino <meshula@hotmail.com>
cary-ilm added a commit that referenced this pull request Feb 16, 2024
* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix typo

Signed-off-by: Cary Phillips <cary@ilm.com>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add copyright notice

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <cary@ilm.com>

* test1

Signed-off-by: Cary Phillips <cary@ilm.com>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <cary@ilm.com>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Bazel support: Bump Imath to 3.1.10 (#1626)

Signed-off-by: Vertexwahn <julian.amann@tum.de>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <cary@ilm.com>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update SECURITY.md

Co-authored-by: Nick Porcino <meshula@hotmail.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <cary@ilm.com>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <cary@ilm.com>

* github security advisory

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention exrcheck

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Nick Porcino <meshula@hotmail.com>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Remove snyk-scan-pr.yml (#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix issue with unpacking sample counts (#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* adjust checks for core to better match c++ checks (#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Fix install of symlink (#1633)

PR #1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <cary@ilm.com>

* adds a shortcut to avoid reconstructing every call (#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* check and control reduceMemory and reduceTime in stream mode (#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in #1634

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add sdist

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Co-authored-by: Kimball Thurston <kdt3rd@gmail.com>
Co-authored-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Nick Porcino <meshula@hotmail.com>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 25, 2024
## Version 3.2.2 (February 11, 2024)

Patch release that addresses
[CVE-2023-5841](https://takeonme.org/cves/CVE-2023-5841.html).

Note that this bug is present in the C++ API (since v3.1.0), although
it is in a routine that is predominantly used for development and
testing. It is not likely to appear in production code.

This release also addresses:

* OSS-fuzz [66491](https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66491)
Out-of-memory in openexr_exrcorecheck_fuzzer
* OSS-fuzz [66489](https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66489)
Null-dereference in `Imf_3_3::realloc_deepdata`

### Merged Pull Requests

* [1632](AcademySoftwareFoundation/openexr#1632)
adjust checks for core to better match c++ checks
* [1630](AcademySoftwareFoundation/openexr#1630)
fix issue with unpacking sample counts
* [1627](AcademySoftwareFoundation/openexr#1627)
Fix CVE 2023 5841
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Mar 3, 2024
…1629)

* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix typo

Signed-off-by: Cary Phillips <cary@ilm.com>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add copyright notice

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <cary@ilm.com>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <cary@ilm.com>

* test1

Signed-off-by: Cary Phillips <cary@ilm.com>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <cary@ilm.com>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix possible int overflow

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add clarifying comment

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Bazel support: Bump Imath to 3.1.10 (AcademySoftwareFoundation#1626)

Signed-off-by: Vertexwahn <julian.amann@tum.de>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <cary@ilm.com>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update SECURITY.md

Co-authored-by: Nick Porcino <meshula@hotmail.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <cary@ilm.com>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <cary@ilm.com>

* github security advisory

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention exrcheck

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Nick Porcino <meshula@hotmail.com>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

* poke

Signed-off-by: Cary Phillips <cary@ilm.com>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <cary@ilm.com>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

* Remove snyk-scan-pr.yml (AcademySoftwareFoundation#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix issue with unpacking sample counts (AcademySoftwareFoundation#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* adjust checks for core to better match c++ checks (AcademySoftwareFoundation#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Fix install of symlink (AcademySoftwareFoundation#1633)

PR AcademySoftwareFoundation#1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <cary@ilm.com>

* adds a shortcut to avoid reconstructing every call (AcademySoftwareFoundation#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* check and control reduceMemory and reduceTime in stream mode (AcademySoftwareFoundation#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in AcademySoftwareFoundation#1634

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* Add sdist

Signed-off-by: Cary Phillips <cary@ilm.com>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Signed-off-by: Cary Phillips <cary@ilm.com>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Co-authored-by: Kimball Thurston <kdt3rd@gmail.com>
Co-authored-by: Vertexwahn <julian.amann@tum.de>
Co-authored-by: Nick Porcino <meshula@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants