[proxy] web.archive.org — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
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

Jump to bottom

bpo-37977: Warn more strongly and clearly about pickle security #15595

Merged
merged 1 commit into from Aug 31, 2019
Merged

bpo-37977: Warn more strongly and clearly about pickle security #15595

merged 1 commit into from Aug 31, 2019

Conversation

Copy link
Contributor

lordmauve commented Aug 29, 2019 •
edited by bedevere-bot

Rewrite the red warning at the top of the pickle module documentation.

  • Simpler, more direct English.
  • Explain the severity of vulnerability that doing this will cause.
  • Link to the hmac module which can be used to prevent tampering.
  • Link to the json module which is safer if less powerful.

https://bugs.python.org/issue37977

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Aug 29, 2019
lordmauve force-pushed the big-red-pickle-warning branch 2 times, most recently from 41f327d to 07e1e09 Compare Aug 29, 2019
serhiy-storchaka requested review from pitrou and tiran Aug 29, 2019
Copy link
Member

pitrou commented Aug 29, 2019

There is already a section titled "Comparison with json", so perhaps move the sentence stating json is secure there?

Copy link
Contributor Author

lordmauve commented Aug 29, 2019

Hmm, good point. I don't think we should move it there because it wouldn't form part of the security warning, but we should link to that section - and that section should state again that JSON doesn't have the arbitrary code execution issue.

rhettinger reviewed Aug 29, 2019
View changes
Doc/library/pickle.rst Outdated
Consider signing data with :mod:`hmac` if you need to ensure that it has not
been tampered with.

The JSON data interchange format provided by the :mod:`json` module is more
Copy link
Contributor

rhettinger Aug 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend leaving this out. It goes well past giving an informative warning and moves into editorializing. Also, for many uses of pickle, JSON is no substitute.

Copy link
Contributor Author

lordmauve Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed this sentence to be less discursive and simply point to the json module and the "Comparison with json" section lower down.

rhettinger self-assigned this Aug 29, 2019
Warn more strongly and clearly about pickle security
ffa614e 
Mention also that JSON is not usually subject to ACE vulnerabilities,
and link to the Comparison with JSON section.
rhettinger merged commit daa82d0 into python:master Aug 31, 2019
Copy link
Contributor

miss-islington commented Aug 31, 2019

Thanks @lordmauve for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

Copy link
Contributor

miss-islington commented Aug 31, 2019

Sorry @lordmauve and @rhettinger, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker daa82d019c52e95c3c57275307918078c1c0ac81 3.8

Copy link
Contributor

miss-islington commented Aug 31, 2019

Thanks @lordmauve for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

Copy link

bedevere-bot commented Aug 31, 2019

GH-15629 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 31, 2019
bpo-37977: Warn more strongly and clearly about pickle security (pyth…
7b30123 
…onGH-15595)

(cherry picked from commit daa82d0)

Co-authored-by: Daniel Pope <lordmauve@users.noreply.github.com>
rhettinger pushed a commit that referenced this pull request Aug 31, 2019
bpo-37977: Warn more strongly and clearly about pickle security (GH-1…
6922b9e 
…5595) (GH-15629)

(cherry picked from commit daa82d0)

Co-authored-by: Daniel Pope <lordmauve@users.noreply.github.com>
Copy link
Contributor Author

lordmauve commented Aug 31, 2019

Thanks, everyone!

lordmauve deleted the big-red-pickle-warning branch Aug 31, 2019
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
bpo-37977: Warn more strongly and clearly about pickle security (pyth…
a2ed2a8 
…onGH-15595)
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
bpo-37977: Warn more strongly and clearly about pickle security (pyth…
9e33c10 
…onGH-15595)
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
bpo-37977: Warn more strongly and clearly about pickle security (pyth…
fdda93f 
…onGH-15595)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

moreati

disconnect3d

rhettinger

pitrou

tiran

Assignees

rhettinger

Labels
docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants