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

Update dependencies for CI/safety failure #5836

Closed
kushaldas opened this issue Mar 2, 2021 · 5 comments · Fixed by #5837
Closed

Update dependencies for CI/safety failure #5836

kushaldas opened this issue Mar 2, 2021 · 5 comments · Fixed by #5837

Comments

@kushaldas
Copy link
Contributor

From https://app.circleci.com/pipelines/github/freedomofpress/securedrop/2045/workflows/84bd1455-d189-49cd-8120-595208047b9c/jobs/51721

^@^@WARNING: You are using pip version 19.3.1; however, version 20.3.4 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
make: Entering directory '/home/circleci/project'
/opt/venvs/securedrop-app-code/bin/safety
███ Running safety...
Checking file ./admin/requirements.txt
safety report
checked 12 packages, using free DB (updated once a month)
---
-> cryptography, installed 3.2.1, affected <3.3.2, id 39606
In the cryptography package before 3.3.2 for Python, certain sequences of update calls to symmetrically encrypt multi-GB values could result in an integer overflow and buffer overflow, as demonstrated by the Fernet class. See CVE-2020-36242.
--
-> jinja2, installed 2.10.1, affected >=0.0.0,<2.11.3, id 39525
This affects the package jinja2 from 0.0.0 and before 2.11.3. The ReDOS vulnerability of the regex is mainly due to the sub-pattern [a-zA-Z0-9._-]+.[a-zA-Z0-9._-]+ This issue can be mitigated by Markdown to format user content instead of the urlize filter, or by implementing request timeouts and limiting process memory. See CVE-2020-28493.
--
-> pyyaml, installed 5.3.1, affected <5.4, id 39611
A vulnerability was discovered in the PyYAML library in versions before 5.4, where it is susceptible to arbitrary code execution when it processes untrusted YAML files through the full_load method or with the FullLoader loader. Applications that use the library to process untrusted input may be vulnerable to this flaw. This flaw allows an attacker to execute arbitrary code on the system by abusing the python/object/new constructor. This flaw is due to an incomplete fix for CVE-2020-1747. See CVE-2020-14343.
--
Makefile:127: recipe for target 'safety' failed
make: *** [safety] Error 1
make: Leaving directory '/home/circleci/project'

Exited with code exit status 2

Note:

@kushaldas
Copy link
Contributor Author

I tried to update both PyYAML and Jinja2 but got stuck in the following error:

✦ ❯ make update-python3-requirements
███ Updating Python 3 requirements files...
Could not find a version that matches pyyaml>=5.4.1 (from -r ../admin/requirements.in (line 3))
Tried: 3.10, 3.10, 3.11, 3.11, 3.12, 3.12, 3.13, 5.1, 5.1.1, 5.1.2, 5.2, 5.3, 5.3.1
Skipped pre-versions: 3.13b1, 3.13rc1, 4.2b1, 4.2b2, 4.2b4, 5.1b1, 5.1b3, 5.1b5, 5.2b1, 5.3b1
There are incompatible versions in the resolved dependencies:
  pyyaml>=5.4.1 (from -r ../admin/requirements.in (line 3))
  pyyaml>=5.4.1 (from -r requirements/python3/develop-requirements.in (line 33))
make: *** [Makefile:31: update-python3-requirements] Error 2

Maybe a fresh set of eyes will help.

@rmol
Copy link
Contributor

rmol commented Mar 2, 2021

Most of the dependencies with safety warnings now require Python 3.6. My current plan is to update Jinja2, which thankfully still supports 3.5, and tell safety to --ignore the other warnings for now (they're either dev dependencies, or we believe our usage is not affected). I was planning to look into environment markers or splitting the requirements files to allow us to use the current versions with Focal. Since you've completed the Jinja2 diff review, I'll start on that.

@emkll
Copy link
Contributor

emkll commented Mar 2, 2021

Thanks for opening the issue @kushaldas and for the investigation @rmol . If we are to hold back / ignore certain vulnerabilities for Xenial (or also other platforms), we should ensure that we are either not affected by the vulnerability or mitigate it in some way. We should also document the rationale for ignoring, in other words why SecureDrop isn't affected by the vulnerability (either in the issue, commit or PR).

@rmol
Copy link
Contributor

rmol commented Mar 2, 2021

Environment markers would allow us to maintain one set of input requirements files, and that would reduce divergence in the set of requirements used on different Python versions, but we'll still need separate outputs to use when building the securedrop-app-code package.

Of the current safety warnings:

Dependency Problem Our version Fixed version Fixed version Python requirement Recommendation
cryptography 39252: Cryptography 3.3 no longer allows loading of finite field Diffie-Hellman parameters of less than 512 bits in length. This change is to conform with an upcoming OpenSSL release that no longer supports smaller sizes. These keys were already wildly insecure and should not have been used in any application outside of testing. 3.2.1 3.3 3.6 We're already ignoring this one, as we're not using those.
cryptography 39606: In the cryptography package before 3.3.2 for Python, certain sequences of update calls to symmetrically encrypt multi-GB values could result in an integer overflow and buffer overflow, as demonstrated by the Fernet class. See CVE-2020-36242. 3.2.1 3.3.2 3.6 I think we can ignore this one. We do use update when writing a SecureTemporaryFile for incoming submissions, but with very small chunks. The largest file we accept is 512 megabytes, so even if it were fed to update whole it wouldn't trigger this bug.
jinja2 39525: This affects the package jinja2 from 0.0.0 and before 2.11.3. The ReDOS vulnerability of the regex is mainly due to the sub-pattern [a-zA-Z0-9.-]+.[a-zA-Z0-9.-]+ This issue can be mitigated by Markdown to format user content instead of the urlize filter, or by implementing request timeouts and limiting process memory. See CVE-2020-28493. 2.10.1 2.11.3 3.5 We don't use urlize, but since it can be upgraded on Xenial, let's.
pylint 39621: "Pylint 2.7.0 includes a fix for vulnerable regular expressions in 'pyreverse'." [Certain strings could cause a DOS because of an inefficient regular expression.] 2.5.0 2.7.0 3.6 I think it's safe to ignore. It's dev-only and we're linting our own code.
pyyaml 39611: A vulnerability was discovered in the PyYAML library in versions before 5.4, where it is susceptible to arbitrary code execution when it processes untrusted YAML files through the full_load method or with the FullLoader loader. Applications that use the library to process untrusted input may be vulnerable to this flaw. This flaw allows an attacker to execute arbitrary code on the system by abusing the python/object/new constructor. This flaw is due to an incomplete fix for CVE-2020-1747. See CVE-2020-14343. 5.3.1 5.4 3.6 We could perhaps use environment markers to update the package in the admin requirements, and ignore it in the others. PyYAML is only in the development requirements and is not included in the securedrop-app-code package.

@zenmonkeykstop
Copy link
Contributor

That's a great analysis @rmol - if the new un-upgradeable dependencies are dev-only I think we can get away with ignores for now.

@rmol rmol mentioned this issue Mar 2, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants