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

pip does not check hashes in wheel RECORD contents during installation #4705

Open
kwlzn opened this issue Sep 1, 2017 · 4 comments
Open
Labels
state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior type: security Has potential security implications

Comments

@kwlzn
Copy link

kwlzn commented Sep 1, 2017

  • Pip version: 9.0.1
  • Python version: 2.7 and higher
  • Operating system: Linux and likely others

Description:

Currently, wheels that fail to install when handled by the wheel library or cli tool due to checksum failures are silently accepted by pip. This seems like a security issue, as pip will gladly and silently install crudely tampered wheels.

Per PEP-427, this hash verification should be happening every time the wheel is extracted for installation: https://www.python.org/dev/peps/pep-0427/#the-dist-info-directory

What I've run:

demonstrating the malformed wheel failure with wheel:

[omerta ~]$ docker run --rm -i -t quay.io/pypa/manylinux1_x86_64 /bin/sh -i
sh-3.2# wget -q http://download.pytorch.org/whl/cu80/torch-0.2.0.post3-cp27-cp27mu-manylinux1_x86_64.whl
sh-3.2# /opt/python/cp27-cp27mu/bin/wheel install ./torch-0.2.0.post3-cp27-cp27mu-manylinux1_x86_64.whl --force
Traceback (most recent call last):
  File "/opt/python/cp27-cp27mu/bin/wheel", line 11, in <module>
    sys.exit(main())
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/wheel/tool/__init__.py", line 356, in main
    args.func(args)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/wheel/tool/__init__.py", line 299, in install_f
    args.wheel_dirs, args.force, args.list_files)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/wheel/tool/__init__.py", line 221, in install
    wf.install(force=force)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/wheel/install.py", line 351, in install
    shutil.copyfileobj(source, destination)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/shutil.py", line 49, in copyfileobj
    buf = fsrc.read(length)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/zipfile.py", line 632, in read
    data = self.read1(n - len(buf))
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/zipfile.py", line 692, in read1
    self._update_crc(data, eof=eof)
  File "/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/wheel/install.py", line 463, in _update_crc
    raise BadWheelFile("Bad hash for file %r" % ef.name)
wheel.install.BadWheelFile: Bad hash for file 'torch/lib/libTHS.so.1'

vs pip behavior:

[omerta pants (master)]$ docker run --rm -i -t quay.io/pypa/manylinux1_x86_64 /bin/sh -i
sh-3.2# /opt/python/cp27-cp27mu/bin/pip --version
pip 9.0.1 from /opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages (python 2.7)
sh-3.2# /opt/python/cp27-cp27mu/bin/pip install http://download.pytorch.org/whl/cu80/torch-0.2.0.post3-cp27-cp27mu-manylinux1_x86_64.whl
Collecting torch==0.2.0.post3 from http://download.pytorch.org/whl/cu80/torch-0.2.0.post3-cp27-cp27mu-manylinux1_x86_64.whl
  Downloading http://download.pytorch.org/whl/cu80/torch-0.2.0.post3-cp27-cp27mu-manylinux1_x86_64.whl (487.1MB)
    100% |████████████████████████████████| 487.1MB 44.7MB/s 
Collecting numpy (from torch==0.2.0.post3)
  Downloading numpy-1.13.1-cp27-cp27mu-manylinux1_x86_64.whl (16.6MB)
    100% |████████████████████████████████| 16.6MB 88kB/s 
Collecting pyyaml (from torch==0.2.0.post3)
  Downloading PyYAML-3.12.tar.gz (253kB)
    100% |████████████████████████████████| 256kB 4.9MB/s 
Building wheels for collected packages: pyyaml
  Running setup.py bdist_wheel for pyyaml ... done
  Stored in directory: /root/.cache/pip/wheels/2c/f7/79/13f3a12cd723892437c0cfbde1230ab4d82947ff7b3839a4fc
Successfully built pyyaml
Installing collected packages: numpy, pyyaml, torch
Successfully installed numpy-1.13.1 pyyaml-3.12 torch-0.2.0.post3
sh-3.2# find /opt |grep -i torch/lib/libTHS.so.1
/opt/_internal/cpython-2.7.13-ucs4/lib/python2.7/site-packages/torch/lib/libTHS.so.1
@pradyunsg pradyunsg added the type: bug A confirmed bug or unintended behavior label Sep 2, 2017
@hashar
Copy link
Contributor

hashar commented Oct 12, 2017

That looks similar to #3513 . As I understand a wheel as a *.dist-info/RECORD file that holds a list of all files and the sha256 sum. pip gracefully installs wheels even if their METADATA or RECORD are empty.

@chrahunt
Copy link
Member

chrahunt commented Sep 2, 2019

See also #2752 for some discussion.

@uranusjr
Copy link
Member

I think #11762 would close this?

@SpecLad
Copy link
Contributor

SpecLad commented Feb 10, 2023

#11762 checks that the RECORD file is syntactically valid, but not that the hashes actually match the files (although I was going to eventually submit a followup patch that would do that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior type: security Has potential security implications
Projects
None yet
Development

No branches or pull requests

6 participants