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

dependency pillow has a security warning #989

Open
jdoconnor opened this issue Oct 27, 2023 · 2 comments
Open

dependency pillow has a security warning #989

jdoconnor opened this issue Oct 27, 2023 · 2 comments

Comments

@jdoconnor
Copy link

Describe the bug

Error details
Pillow versions before v10.0.1 bundled libwebp binaries in wheels that are vulnerable to GHSA-hhrh-69hc-fgg7 (previously GHSA-j7hp-h8jx-5ppr). Pillow v10.0.1 upgrades the bundled libwebp binary to v1.3.2.

The current version is pinned to
Pillow>=6.2.2,!=9.2.*

@Lucas-C
Copy link
Member

Lucas-C commented Oct 30, 2023

Thank you for the report @jdoconnor

I'm not sure that we should address this in fpdf2.

Let me explain my thought process and I'd be happy to get feedback on it 😊

The install_requires in https://github.com/py-pdf/fpdf2/blob/master/setup.cfg#L43 serves to define the minimal version of dependencies required by fpdf2 so that this library can be functional.

I don't think that this precise setup.cfg entry in fpdf2 should be altered to prevent users using versions of other libraries with known vulnerabilities, because this is a minimal version (if we pinned exact versions, of course we should bump them when vulnerabilities are found).

When a user install fpdf2 using pip (or other dependency managers like poetry),
the latest compatible version of fpdf2 dependencies will be installed,
so the latest (non-vulnerable) Pillow package will be installed.

Of course, there is also the scenario where users use some kind of package lock mechanism,
either poetry.lock files, Pipfile.lock, or a requirements.txt with fixed versions.
But in that case, upgrading Pillow minimal version to v10.0.1 in fpdf2 install_requires won't be useful neither!

Tools like renovate or snyk can be used to detect the presence of libs with known vulnerabilities in package lock files (or simply enforce up-to-date dependencies in lock files), and they are great for this!

But I don't see how bumping Pillow minimal version to v10.0.1 in fpdf2 install_requires would be useful in any way...

Moreover, this could cause trouble for end users that have constraints on their execution environment.
For example, a user with a limited choice of usable Pillow versions (maybe due to the necessity to use an old Python interpreter), would be really annoyed to have this version constraint of Pillow>=10.0.1 in fpdf2.

What do you think @jdoconnor, @gmischler, @andersonhc?

@gmischler
Copy link
Collaborator

When a user install fpdf2 using pip (or other dependency managers like poetry),
the latest compatible version of fpdf2 dependencies will be installed,
so the latest (non-vulnerable) Pillow package will be installed.

Several scenarios are possible on a user's system (assuming the minimum Pillow version is not bumped):

  1. Neither the dependency nor fpdf2 is present, and fpdf2 gets newly installed. The installation happens as you describe.
  2. A vulnerable but supported dependency is already present and fpdf2 gets newly installed. What happens to the dependency?
  3. Both a vulnerable but supported dependency and an older fpdf2 version are present, and fpdf2 gets updated. What happens to the dependency?

I'd expect the last case to be the most common one.

For example, a user with a limited choice of usable Pillow versions (maybe due to the necessity to use an old Python interpreter)

With users like that, the likelyhood that they understand how to use a custom dependency file seems rather high.

What do you think

The CVE is rated at a significant severity, since it makes it possible to execute arbitrary code on the target system, just by providing a manipulated image file. Since fpdf2 allows to import image files from arbitrary URLs, that gives us quite some responsibility to close the gap.

@Lucas-C Lucas-C removed the bug label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants