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

Document security expectations #1623

Merged

Conversation

cary-ilm
Copy link
Member

@cary-ilm cary-ilm commented Feb 1, 2024

No description provided.

Signed-off-by: Cary Phillips <[email protected]>

The only external library dependency is
[libdeflate](https://github.com/ebiggers/libdeflate), which implements
standard deflate/zlib/gzip compression and decompression.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does Imath not count as an 'external library dependency'?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just for the records: c-blosc2 could also become a dependency of OpenEXR soon, which has dependencies to lz4, zlib-ng, and zstd.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was considering Imath a part of the "OpenEXR Project", but yes, better to call that out explicitly.

And yes, we'll need to keep this statement up to date if/when we add dependencies.

Signed-off-by: Cary Phillips <[email protected]>
SECURITY.md Outdated
Comment on lines 127 to 129
The primary threat to OpenEXR is software faults in the form of heap
buffer overflows, out-of-memory faults, or segmentation faults that
could be exploitable as denial-of-service attacks.
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the threat model would more be looking at trust boundaries, inputs, outputs etc. What's described here is more potential vulnerabilities.

The goal of a threat model and threat analysis is to really analyze how a pice of software of system could be used in malicious or non expected ways, see if there is enough protection in place and how possible issues could/can be mitigated, how things could be exploited and what would happen if something bad was to happen. It's also used as a way to weight the severity and likelihood of possible defects, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to work up some verbiage with the help of AI, and the result looks good to me. I am not sure if you are suggesting we should describe the threat model better, which I have requested of the AI, or if you are suggesting we should undertake an analysis. If the latter, I would counter that it is perhaps out of scope for this PR, as that would be, if done properly, a very large project. Perhaps we could refine this with more specific facts about OpenEXR and less of the generalities the AI has proposed from general principles.


The threat model of OpenEXR, a high-dynamic-range (HDR) image file format, primarily revolves around potential vulnerabilities that could compromise the confidentiality, integrity, or availability of data stored in OpenEXR files. While OpenEXR itself is a well-established and widely used format, like any software, it may have certain considerations in terms of security. Here are some aspects to consider:

Trust Boundaries:

Identify and define trust boundaries within the OpenEXR ecosystem. This includes interactions with external systems, libraries, and user inputs.
Evaluate the trustworthiness of components interacting with OpenEXR, considering factors such as data sources, processing modules, and external dependencies.

Inputs and Outputs:

Analyze the types of inputs OpenEXR processes and ensure robust validation mechanisms are in place to handle potentially malicious or unexpected input.
Assess the security of output data to prevent information disclosure, unintended leakage, or exploitation of sensitive information.

Misuse Scenarios:

Enumerate potential misuse scenarios, considering both intentional and unintentional misuse of OpenEXR functionality.
Evaluate the impact of scenarios where OpenEXR is used as part of an attack chain or where it unintentionally facilitates malicious activities.

Protection Mechanisms:

Evaluate existing protection mechanisms to determine their effectiveness in mitigating identified threats.
Identify areas where additional security measures, such as input validation, encryption, or access controls, may be necessary to enhance the overall security posture.

Severity and Likelihood Assessment:

Weigh the severity and likelihood of identified threats to prioritize mitigation efforts. Consider the impact of successful exploitation and the likelihood of occurrence.

Defects and Vulnerabilities:

Conduct a thorough analysis of potential defects and vulnerabilities in the OpenEXR software, assessing their potential impact on the overall security of the system.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry if my comment wasn't clear. I'm not suggesting to do a threat model and/or threat analysis. I am more pointing to the fact that the current paragraph isn't describing a threat model per say and I'm describing what a threat model is.

As for the output from the AI, it's a little bit vague and not very useful I think. It's basically describing what a threat model is and how to write/define one and not what the threat model is. But it has some bits that could be useful.

So how about maybe something like:

The entry point is any images being loaded using the library. Malformed images could caused issues (list issues). OpenEXR has tests (fuzz tests, unit tests, etc) to mitigate against these. (does it also set sane default compiler flags to protect against some of these errors?), etc...

Would that make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure the AI blurb really says anything significant. I change "Thread Model" to "Potential Vulnerabilities" and lifted @JeanChristopheMorinPerso verbiage, thanks. The next section mentions tests, this is just to state what the vulnerabilities are.

This is my take on what's required for the "security expectations", but it's not (yet) the "assurance case" the badgeapp also requires, that's for later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for proposing some text @JeanChristopheMorinPerso, I was puzzling over what we could possibly write there, so thought maybe the AI quote would help jostle up some ideas.

Choose a reason for hiding this comment

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

The changes look good to me!

SECURITY.md Outdated Show resolved Hide resolved
cary-ilm and others added 3 commits February 2, 2024 11:38
@cary-ilm
Copy link
Member Author

cary-ilm commented Feb 7, 2024

I've incorporated the above suggestions, look ok now?

SECURITY.md Outdated
post patches as quickly as possible. If you do not get a response to
a message sent to [email protected] within 48 hours, contact the
project maintainers via a GitHub
[Issue](https://github.com/AcademySoftwareFoundation/openexr/issues).

Choose a reason for hiding this comment

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

Could I suggest https://github.com/AcademySoftwareFoundation/openexr/security/advisories/new instead of issue if you don't want the issues to be public? See https://docs.github.com/en/enterprise-cloud@latest/code-security/security-advisories/working-with-repository-security-advisories/about-repository-security-advisories for more info. Unless you want potential security issues to be disclosed right away, in which case github issues would work.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes a lot of sense to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion, I added GitHub security advisories as the preferred method for reporting vulnerabilities, with email as an alternative, but also mentioned that if you don't get a response, your address may be blocked (it's happened).

Choose a reason for hiding this comment

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

It looks great!

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm
Copy link
Member Author

cary-ilm commented Feb 8, 2024

I've added a couple more notes, I'm going to merge this as is and we can follow up with any further edits later

@cary-ilm cary-ilm merged commit a3fe0eb into AcademySoftwareFoundation:main Feb 8, 2024
2 checks passed
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 11, 2024
* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
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 <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

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

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* 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 <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

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

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
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 <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

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

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* 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 <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

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

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
cary-ilm added a commit that referenced this pull request Feb 16, 2024
* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
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 <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

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

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* 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 <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

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

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
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 <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

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

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* 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 <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

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

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* 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 <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

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

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
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.

5 participants