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

(unused) vendored requests is vulnerable to CVE-2018-18074 #1608

Closed
asottile opened this issue Nov 16, 2018 · 12 comments · Fixed by #1829
Closed

(unused) vendored requests is vulnerable to CVE-2018-18074 #1608

asottile opened this issue Nov 16, 2018 · 12 comments · Fixed by #1829
Labels
dependencies This issue is a problem in a dependency.

Comments

@asottile
Copy link

Assuming this history is correct, it is currently vendored at 2.7.0

Versions prior to 2.20.0 are vulnerable to this

See CVE-2018-18074

This vendored copy is not used by botocore itself any more, though some downstream libraries (such as pynamodb) are reaching into botocore's vendor directory and using it

@joguSD
Copy link
Contributor

joguSD commented Nov 19, 2018

Thanks for opening this issue, as you mention we no longer use this vendored version of requests directly in the SDK and have kept in the code base for backwards compatibility. Customers using the latest version of the SDKs are unaffected.

We're currently investigating options to protect customers that might be using the vendored version of requests.

@joguSD joguSD added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Nov 19, 2018
@ztou
Copy link

ztou commented Nov 27, 2018

@joguSD : not sure about what is no longer use mean, if I search like: https://github.com/boto/botocore/search?l=Python&p=2&q=vendored, we can see there are lots of code still reference this vendored request, and we can't simple remove that folder manually even in the latest code base.

sample error:

  File "/venv/lib/python2.7/site-packages/botocore/exceptions.py", line 15, in <module>
    from botocore.vendored import requests
ImportError: cannot import name requests

@asottile
Copy link
Author

@ztou the only use is exception base classes (which can pretty easily be switched) -- the actual requesting parts of request are unused (where the CVE lives)

@tmclaugh
Copy link

tmclaugh commented Jan 7, 2019

There's a bit of common wisdom out there to use from botocore.vendored import requests in your own application code. For AWS Lambda, it's one less dependency to package.

EDIT: Just seeing now that you announced deprecation in April 2018.

@asottile
Copy link
Author

asottile commented Jan 7, 2019

Same number of deps just one is sneaky ;) I'd be hard pressed to call it "wisdom" - - seems foolish to import from another module's compat / vendor modules and expect a stable api

@joguSD
Copy link
Contributor

joguSD commented Jan 7, 2019

Just as an update we've added deprecation warnings to real usage of the vendored requests package in this pr. In the long term, we're hoping to be able to remove the package entirely (or as much of it as possible).

@tmclaugh That usage pattern will almost certainly run into issues and we strongly recommend that people not use our vendored version of requests.

@asottile
Copy link
Author

would it make sense to warnings.warn(FutureWarning, ...) in botocore/vendored/__init__.py?

@JordonPhillips JordonPhillips removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Mar 11, 2019
@ghost
Copy link

ghost commented Mar 13, 2019

Sorry but I fail to see how I am actually "forcing" the use of the vendored library anywhere.
Simple use of botocore insists on using the vendored version of requests which in turn uses vendored version of urllib3 and it all falls to pieces.

@selik
Copy link

selik commented Mar 19, 2019

@asottile, exception bases classes are not the only usage if you include the tests. There are a number of uses of vendored.requests.get in test_s3.py.

@ztou the only use is exception base classes (which can pretty easily be switched) -- the actual requesting parts of request are unused (where the CVE lives)

@asottile
Copy link
Author

@asottile, exception bases classes are not the only usage if you include the tests. There are a number of uses of vendored.requests.get in test_s3.py.

@ztou the only use is exception base classes (which can pretty easily be switched) -- the actual requesting parts of request are unused (where the CVE lives)

tests aren't distributed with the package

@ghost
Copy link

ghost commented Apr 3, 2019

Just following up on this one.
I figured out that the vendored packaged, although not generally used, are in fact loaded when relying on exception. This loads the vendored requests package which in turn loads vendored urillib3 with outdated versions.
So there is no special loading of libraries to make this error appear.

@jamesls
Copy link
Member

jamesls commented Sep 20, 2019

Hi, we are planning to remove the vendored version of requests in one month. Please let us know if you have any questions or concerns in the tracking PR: #1829

@joguSD joguSD added the dependencies This issue is a problem in a dependency. label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies This issue is a problem in a dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants