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

[CVE RISK] Upgrade urllib3 to v1.26.5 #378

Closed
IanHoang opened this issue Sep 27, 2023 · 14 comments
Closed

[CVE RISK] Upgrade urllib3 to v1.26.5 #378

IanHoang opened this issue Sep 27, 2023 · 14 comments
Assignees
Labels

Comments

@IanHoang
Copy link
Collaborator

The Issue

An issue was discovered in urllib3, when provided with a URL containing many @ characters in the authority component, the authority regular expression exhibits catastrophic backtracking, causing a denial of service if a URL were passed as a parameter or redirected to via an HTTP redirect.

An issue was discovered in urllib3 before 1.26.5. When provided with a URL containing many @ characters in the authority component, the authority regular expression exhibits catastrophic backtracking, causing a denial of service if a URL were passed as a parameter or redirected to via an HTTP redirect.

How to Fix

The following branches contain urllib3 v1.25.11 and need to be upgraded to v1.26.5 or later.

  • origin/main
  • origin/1
  • origin/1.1
@IanHoang IanHoang added bug Something isn't working untriaged good first issue Good for newcomers High Priority and removed bug Something isn't working untriaged labels Sep 27, 2023
@AkshathRaghav
Copy link
Contributor

AkshathRaghav commented Sep 28, 2023

From my understanding, setup.py in the opensearch-benchmark branches installs opensearch-py (different versions) which have "urllib3>=1.21.1" in install_requires function of setup.py.

Should I add "urllib3>=1.26.5" in install_requires of opensearch-py or opensearch-benchmark?

@maddox05
Copy link
Contributor

As I see it opensearch-py requires urllib3 and downloads it, @AkshathRaghav, I think doing either would work. I think adding urllib3>=1.26.5 on setup.py (depending on how it's installed) should then update it to >1.26.5. Although I think adding it in OpenSearch-py would be the better option.

@IanHoang
Copy link
Collaborator Author

IanHoang commented Sep 28, 2023

@AkshathRaghav Either would work as @maddox05 mentioned. Feel free to make a PR request to opensearch-py to see if they'd be willing to bump urllib3 from v1.21.1 to v1.26.5

https://github.com/opensearch-project/opensearch-py/blob/main/setup.py#L53

If they provide a reason why they cannot, we can opt to add urllib3>=1.26.5 to our own setup.py

@maddox05
Copy link
Contributor

maddox05 commented Sep 29, 2023

@IanHoang will do, I'll keep you updated here. nvm @AkshathRaghav got it 👏

@AkshathRaghav
Copy link
Contributor

Pull request to the main branch - opensearch-project/opensearch-py#515

Waiting for the checks to pass before I do the PRs for the other version branches in opensearch-py (which branch 1 and 1.1 here depend on)

@maddox05
Copy link
Contributor

@AkshathRaghav possibly hardcoding it in

  • origin/main
  • origin/1
  • origin/1.1
    wouldn't hurt.

@AkshathRaghav
Copy link
Contributor

AkshathRaghav commented Sep 29, 2023

Sure that'd be easier tbh. But when setting it up, it'd install twice.

Both methods are low effort so no worries!

@maddox05
Copy link
Contributor

maddox05 commented Sep 29, 2023

it should be installed through opensearch library with the first command, and then if its already installed it should just skip it. it just acts as a backup in case opensearch-py library installed the wrong ver of urllib3.

@AkshathRaghav
Copy link
Contributor

AkshathRaghav commented Sep 29, 2023

If the main opensearch-py setup has a dependency as 1.26.5, it can't install the wrong version; their setup.py will only install it if the module wasn't existing.

@maddox05
Copy link
Contributor

that is what I meant yes sorry for the confusion.

@wbeckler
Copy link

wbeckler commented Oct 3, 2023

Why not bump the opensearch-py version once #378 happens?

Edit, I meant opensearch-project/opensearch-py#518

@IanHoang
Copy link
Collaborator Author

IanHoang commented Oct 3, 2023

@wbeckler our setup.py's install_requires doesn't specify urllib3 as a dependency since it is installed with opensearch-py. Instead of explicitly specifying urllib3 in OSB's setup.py, it'll be more straightforward for OSB to use the urllib3 that comes packaged withopensearch-py.

Note: opensearch-project/opensearch-py#518 has been merged in

@IanHoang
Copy link
Collaborator Author

IanHoang commented Oct 3, 2023

opensearch-py is considering releasing version 2.3.2 with the urllib3 v1.26.9 incorporated. After that's been released, @AkshathRaghav or @maddox05 can open a PR to change the opensearch-py version from 2.2.0 to 2.3.2

@AkshathRaghav
Copy link
Contributor

Will do @IanHoang 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

4 participants