Upgrade v2 dependencies to address recent security vulnerabilities like CVE-2024-34069 in werkzeug

[chrisinmtown]

Please patch version 2 security issues for us long-time loyal users! I really do want to upgrade to Connexion v3 but that path is long & windy. Current security vulnerabilities are critical.

Description

Connexion 2.x with Flask is flagged for vulnerabilities in the werkzeug library version < 3.0.6:

• Werkzeug debugger vulnerable to remote execution when interacting with attacker controlled domain, CVE-2024-34069, GHSA-2g68-c3qc-8985
• Werkzeug DoS: High resource usage when parsing multipart/form-data containing a large part with CR/LF character at the beginning, CVE-2023-46136, GHSA-hrfv-mqp8-q5rw
• Applications using Werkzeug to parse multipart/form-data requests are vulnerable to resource exhaustion. A specially crafted form body can bypass the Request.max_form_memory_size setting, CVE-2024-49767, GHSA-q34m-jh98-gwm2

The version that addresses al problems is 3.0.6, but that is a disruptive change. A version that addresses the 2023 CVS is 2.3.8.

Expected behaviour

Security scans of Connexion/Flask dependencies yield no CVE numbers

Actual behaviour

Security scans of Connexion/Flask dependencies yield the issues listed above

Steps to reproduce

Let the Github security check run on a requirements.txt file

Additional info:

• Python 3.11
• Connexion Version: 2.14.2

Just for the record, here are the dependencies from the Connexion V2 branch, which is what we're running from PyPI as 2.14.2:

install_requires = [
    'clickclick>=1.2,<21',
    'jsonschema>=2.5.1,<5',
    'PyYAML>=5.1,<7',
    'requests>=2.9.1,<3',
    'inflection>=0.3.1,<0.6',
    'werkzeug>=1.0,<2.3',
    'importlib-metadata>=1 ; python_version<"3.8"',
    'packaging>=20',
]

[chrisinmtown]

Kudos to @mfmarche who opened this PR https://github.com/spec-first/connexion/pull/1967/files

[chrisinmtown]

People asked the Werkzeug maintainers for a fix to the 2024 CVE in their v2 release, and they declined. They explained that the vulnerability can be completely avoided by explicitly disabling the debugger. That may be a viable workaround. Please see discussion here: pallets/werkzeug#2915

[chrisinmtown]

If you would please consider upgrading the V2 branch to Werkzeug version 2.3.8, even that would be a help, because it would address CVE-2023-46136 at minimal cost & disruption.

[RobbeSneyders]

Please see #1757

I would be happy to accept a PR that makes Connexion 2 compatible with newer Werkzeug versions without making any breaking changes to Connexion itself. Although I have to warn you, that lately every Werkzeug / Flask version contains breaking changes, since they no longer follow semantic versioning.

[chrisinmtown]

Thanks @RobbeSneyders for the warning about Werkzeug. Gotta start somewhere tho, I started by checking out branch "v2" and running tox, with absolutely no version updates at all. The result was not promising. For example:

Error: Invalid value for '--req' / '-r': File 'requirements-devel.txt' does not exist.

And also:

tests/api/test_responses.py:396: error: invalid syntax

Tox is configured to test against python 3.7, 3.8, 3.9; those are a bit out of date at this point. It would help if you can provide guidance. Is that the right branch? Are there some undocumented yet 100% required setup steps? Please comment.

[RobbeSneyders]

The first step would indeed be to get the tests green without any additional changes.

The last v2 release was almost 2 years ago though, so I would not be surprised if the tests are broken due to dependencies. The test setup also changed quite a bit between v2 and v3, so I'm not very familiar anymore with how it was set up for v2.

I just submitted a pipeline to check the v2 tests here and the first issue it runs into is the setuptools version. We are installing it in the github actions, but seems like a different version is installed in the tox environment. So this would be the first thing to fix.

I would be happy to approve workflow runs on your PR so you can test them with github actions so you don't have to worry about your local setup.

[chrisinmtown]

Here's the latest vulnerability identified in Werkzeug: CVE-2024-49767

[chrisinmtown]

Sorry I realized very late that this is basically a dupe of #1958

[chrisinmtown]

I installed python 3.9 and made the following minor changes. This lets the test suite py39-min run to completion successfully, so I thought I'd ask for feedback before going farther, please comment.

I didn't yet attempt to change the version of setuptools - not ignoring you just have not figured it out yet. I see the warning in the GitHub runner output:

   Please remove any references to `setuptools.command.test` in all supported versions of the affected package.

I don't see any way to pin to an old version of setuptools. Maybe that's the wrong thing to do anyhow?

diff --git a/setup.py b/setup.py
index bf66f2e..dc4a6c0 100755
--- a/setup.py
+++ b/setup.py
@@ -25,7 +25,7 @@ install_requires = [
     'PyYAML>=5.1,<7',
     'requests>=2.9.1,<3',
     'inflection>=0.3.1,<0.6',
-    'werkzeug>=1.0,<2.3',
+    'werkzeug>=1.0,<2.4',
     'importlib-metadata>=1 ; python_version<"3.8"',
     'packaging>=20',
 ]
@@ -44,7 +44,7 @@ aiohttp_require = [
 
 tests_require = [
     'decorator>=5,<6',
-    'pytest>=6,<7',
+    'pytest>=6,<9',
     'pytest-cov>=2,<3',
     'testfixtures>=6,<7',
     *flask_require,
diff --git a/tests/api/test_responses.py b/tests/api/test_responses.py
index da76bbf..68b9322 100644
--- a/tests/api/test_responses.py
+++ b/tests/api/test_responses.py
@@ -393,29 +393,29 @@ def test_streaming_response(simple_app):
 def test_oneof(simple_openapi_app):
     app_client = simple_openapi_app.app.test_client()
 
-    post_greeting = app_client.post(  # type: flask.Response
+    post_greeting = app_client.post(
         '/v1.0/oneof_greeting',
         data=json.dumps({"name": 3}),
         content_type="application/json"
-    )
+    )  # type: flask.Response
     assert post_greeting.status_code == 200
     assert post_greeting.content_type == 'application/json'
     greeting_reponse = json.loads(post_greeting.data.decode('utf-8', 'replace'))
     assert greeting_reponse['greeting'] == 'Hello 3'
 
-    post_greeting = app_client.post(  # type: flask.Response
+    post_greeting = app_client.post(
         '/v1.0/oneof_greeting',
         data=json.dumps({"name": True}),
         content_type="application/json"
-    )
+    )  # type: flask.Response
     assert post_greeting.status_code == 200
     assert post_greeting.content_type == 'application/json'
     greeting_reponse = json.loads(post_greeting.data.decode('utf-8', 'replace'))
     assert greeting_reponse['greeting'] == 'Hello True'
 
-    post_greeting = app_client.post(  # type: flask.Response
+    post_greeting = app_client.post(
         '/v1.0/oneof_greeting',
         data=json.dumps({"name": "jsantos"}),
         content_type="application/json"
-    )
+    )  # type: flask.Response
     assert post_greeting.status_code == 400

[chrisinmtown]

I am trying to understand what setup.py is doing by defining class PyTest(TestCommand): which uses the now-deprecated setuptools.command.test.test object; and passing it to the setup() function via cmdclass={'test': PyTest}. I have never used that. Maybe someone who understands the intent here can comment, whether that can be dropped, in favor of a simpler way of invoking pytest?

[chrisinmtown]

For what it's worth, I dug thru the git history in branch v2 and found commit f55cb1c that still has file requirements-devel.txt (deleted in later commits) with the following content. That file is required for environments py37-dev, py38-dev, py39-dev. I'm not at all certain if those environments are important, or if they can be dropped.

-e git+https://github.com/Julian/jsonschema.git@v2.6.0#egg=jsonschema
-e git+https://github.com/pallets/flask.git#egg=flask
-e git+https://github.com/kennethreitz/requests#egg=requests
-e git+https://github.com/zalando/python-clickclick.git#egg=clickclick
inflection
mock
pytest
testfixtures>=5.3.0
# This repo doesn't have the latest version released on PyPI
# -e hg+https://bitbucket.org/pitrou/pathlib#egg=pathlib
# PyYAML is not that easy to build manually, it may fail.
#-e svn+http://svn.pyyaml.org/pyyaml/trunk#egg=PyYAML

[chrisinmtown]

tl;dr - it appears to me that changes in the package setuptools require the old V2 branch of connexion to have rewritten versions of files setup.py and tox.ini, or alternately a new pyproject.toml and a revised tox.ini file . How much work do you think it might be to port the files from main branch (that use poetry) back to v2? Please comment.

[RobbeSneyders]

I recommend running the v2 test suite with the old version of setuptools and other test dependencies and fixing any failing tests. If the tests are green again, you can update the test setup. I would not try to do both at once.

[chrisinmtown]

Ok I will keep trying to run the v2 test suite with the old setuptools. With the patches shown a couple comments above, tox runs py39-min successfully. Tox py39-pypi fails on tests/decorators/test_validation.py::test_invalid_type. I'm not sure how the min dependencies vs the pypi dependencies change how a dict gets serialized, that the keys are not shown in sorted order. Here's the failure from the py39-pypi run, you can see the content is the same but the order is different:

>       assert result == expected_result
E       AssertionError: assert '20 is not of...ance:\n    20' == '20 is not of...ance:\n    20'
E         
E           20 is not of type 'string'
E           
E           Failed validating 'type' in schema:
E         -     {'name': 'foo', 'type': 'string'}
E         +     {'type': 'string', 'name': 'foo'}
E           
E           On instance:
E               20

I found that sorting the tags in the /input/ to the function yields consistent sorted order in the message with both min and pypi dependencies, the diff is below. Please say, is this a genuine failure revealed by using different dependencies, and needs a proper fix, or is it something trivial, and adjusting the test input is sufficient?

diff --git a/tests/decorators/test_validation.py b/tests/decorators/test_validation.py
index 0a4685a..e7d1596 100644
--- a/tests/decorators/test_validation.py
+++ b/tests/decorators/test_validation.py
@@ -62,7 +62,7 @@ def test_get_valid_parameter_with_enum_array_header():
 def test_invalid_type(monkeypatch):
     logger = MagicMock()
     monkeypatch.setattr('connexion.decorators.validation.logger', logger)
-    result = ParameterValidator.validate_parameter('formdata', 20, {'type': 'string', 'name': 'foo'})
+    result = ParameterValidator.validate_parameter('formdata', 20, {'name': 'foo', 'type': 'string'})
     expected_result = """20 is not of type 'string'
 
 Failed validating 'type' in schema:

[mfmarche]

@chrisinmtown see my MR: #1992 . I haven't tested on py311, but I don't anticipate it being a problem.