Skip to content

Blocking of stdlib names has holes? #2940

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

Closed
hartwork opened this issue Feb 14, 2018 · 4 comments · Fixed by #2942
Closed

Blocking of stdlib names has holes? #2940

hartwork opened this issue Feb 14, 2018 · 4 comments · Fixed by #2942
Assignees

Comments

@hartwork
Copy link

hartwork commented Feb 14, 2018

Hi!

Pull request #2409 introduced a blacklist of stdlib names to counter to protect against pytosquatting, aiming to plug #2151.

I found a hole with it. While names like xmlrpc.client and encodings.idna are blocked because they appear in some stdlib-list list, some parent packages like encodings are not in their list and do not get blocked by warehouse as a consequence. I was able to register these four packages:

According to pytosquatting.org, package encodings had about 20 hits per day in the past, not sure how much hits the others would get. Personally, I would have considered at least encodings and xmlrpc worth blocking.

Should stdlib-list be completed or rather parent package names auto-included at warehouse level for a generic fix? What do you think?

PS: I'm happy to delete these four packages from PyPI once this hole is plugged. I would ask to keep them up until then, please feel free to verify their whitehat nature. Thank you!

Best, Sebastian

CC: @benjaoming @hannob

@ewdurbin
Copy link
Member

@hartwork thanks for the report. interesting work around :)

i'm going to get a fix in at the warehouse level, then pull down the registered packages.

in the future, PyPI does have a security policy published at https://pypi.org/security/ and https://pypi.python.org/security for issues relating to the security of our service or our users.

ewdurbin added a commit that referenced this issue Feb 14, 2018
ewdurbin added a commit that referenced this issue Feb 14, 2018
fixes #2940, initial work failed to take this into consideration.
ewdurbin added a commit that referenced this issue Feb 14, 2018
ewdurbin added a commit that referenced this issue Feb 14, 2018
fixes #2940, initial work failed to take this into consideration.
ewdurbin added a commit that referenced this issue Feb 14, 2018
* failing test for #2940

* block top-level module namespaces not covered by stdlib-list

fixes #2940, initial work failed to take this into consideration.

* additional test cases for #2942
@hartwork
Copy link
Author

hartwork commented Feb 14, 2018

Nice! Can I keep the encodings and/or xmlrpc package for a bit? I wanted to show pip install encodings live to a co-worker tomorrow and see his face. This is what you'd see:

# pip install encodings --user
Collecting encodings
  Downloading encodings-0.1.5.tar.gz
Installing collected packages: encodings
  Running setup.py install for encodings ... 
                               ^^^^^^^^^
           _          _           _        _ _
     _ __ (_)_ __    (_)_ __  ___| |_ __ _| | |
    | '_ \| | '_ \   | | '_ \/ __| __/ _` | | |
    | |_) | | |_) |  | | | | \__ \ || (_| | | |
    | .__/|_| .__/   |_|_| |_|___/\__\__,_|_|_|
    |_|     |_|
                           _           _     ___ _
                 __      _| |__   __ _| |_  |__ \ |
                 \ \ /\ / / '_ \ / _` | __|   / / |
     _   _   _    \ V  V /| | | | (_| | |_   |_||_|
    (_) (_) (_)    \_/\_/ |_| |_|\__,_|\__|  (_)(_)

      error
    Complete output from command /usr/bin/python2.7 -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-jGHLI_/encodings/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-YFcVn0-record/install-record.txt --single-version-externally-managed --compile --user --prefix=:
    
    ----------------------------------------
Command "/usr/bin/python2.7 -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-jGHLI_/encodings/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-YFcVn0-record/install-record.txt --single-version-externally-managed --compile --user --prefix=" failed with error code 1 in /tmp/pip-build-jGHLI_/encodings/

@ewdurbin
Copy link
Member

@hartwork no I'm sorry, these packages have already been blocked and deleted.

We probably could have negotiated something if this were under less pressure by being responsibly disclosed.

@pypi pypi locked as resolved and limited conversation to collaborators Feb 14, 2018
@ewdurbin
Copy link
Member

I reacted a bit swiftly in locking this issue. I want to make clear that we value and appreciate reports like these, but would prefer for them to be disclosed per our security policy (also published here).

The realistic outcomes of situations like these can be costly (in time) for the maintainers of the project. Disseminating this sort of information publicly (this issue was tweeted by a prominent security researcher almost immediately after being opened) can lead to others trying out the holes and further complicating the cleanup efforts, regardless of action taken with the packages you had already claimed.

Aside from the pressure of trying to ship this as quickly as possible to avoid additional cleanup, we hit a snag with the Continuous Integration environment that delayed deployment of a fix we deemed critical by nearly 30 minutes. This pressure translates to real stress on real humans.

Responsibly disclosing this issue per our security policy would have greatly reduced this pressure and stress, which ultimately manifested as me punching the lock button to be through with it.

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

Successfully merging a pull request may close this issue.

2 participants