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

pickle is unsafe #30

Closed
fxkr opened this issue Jan 25, 2011 · 4 comments
Closed

pickle is unsafe #30

fxkr opened this issue Jan 25, 2011 · 4 comments

Comments

@fxkr
Copy link

fxkr commented Jan 25, 2011

If the server secret leaks an attacker can forge cookies and get the server to unpickle them, which gives him remote code execution.

Therefore, something else should be used for serialization. Maybe json.

Proof of concept:

Python 2.7.1 (r271:86832, Jan  6 2011, 11:51:37) 
[GCC 4.5.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pickle; pickle.loads("cos\nsystem\n(S'whoami'\ntR.")
fxkr
@mitsuhiko
Copy link
Contributor

Not a bug. Behavior of pickle is known, if one wants to use a different serialization system one can hook in simplejson with a single line change. Just override the serialization_method attribute of the secure cookie with the json module.

@fxkr
Copy link
Author

fxkr commented Jan 26, 2011

How is that not a bug if knowing the server secret which is lying around in plaintext in a config file leads to remote code execution?

This can easily happen on a server with multiple users and incorrectly set file permissions, and I am fairly sure end users (!) will in most cases be unaware of the consequences of server secret compromittation. Ok, in that scenario it wouldn't be called remote code execution but privilege escalation (that other user can escalate to whatever user runs the web application).

I was aware of the fact that one can set serialization_method, but I believe pickle shouldn't be the default.

Your call though.

@mitsuhiko
Copy link
Contributor

Werkzeug itself is compatible down to Python 2.4 where simplejson is not bundled. Thus that can't be the default in the first place. It would be an option to make it the default for Flask, that however would mean breaking backwards compatibility.

If anyone has access to your server files you have a few other issues as well. What I will do however is adding a warning to the documentation.

@fxkr
Copy link
Author

fxkr commented Jan 26, 2011

Thanks.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants