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

Consider warning about pickle code execution security aspects #13

Open
hartwork opened this issue Mar 8, 2017 · 12 comments
Open

Consider warning about pickle code execution security aspects #13

hartwork opened this issue Mar 8, 2017 · 12 comments

Comments

@hartwork
Copy link

hartwork commented Mar 8, 2017

Hi!

I noticed the pickle aspect of tblib and I am concerned that (direct or indirect) users of tblib may miss that unpickling data by other people puts them in a remote-code-execution position. Would you be in with adding a big fat warning that you should only use tblib on pickle files you produced yourself or produced by software where you have sole control over all nodes producing these files?

Thanks for your consideration, best

Sebastian

@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

I could add some warnings, but the use of pickling is already mentioned. What's the catch here? Should we change all the projects that use pickle to warn users that pickling has limitations? Isn't the python docs/tutorials a better place to educate users about pickling?

ionelmc added a commit that referenced this issue Mar 8, 2017
@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

"Limitations" doesn't really hit the spot for arbitrary code execution, I would say.
The python docs do have a warning but it has little chance to do what an article like https://lincolnloop.com/blog/playing-pickle-security/ does. So personally I think it would be nice to have tblib warn about it too or maybe even link to that (or a similar) article, yes.

While at it, do you have to rely on pickle for tblib or could you do the same with something more safe?

@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

I think there's some confusion here or you haven't actually looked at tblib's API at all :)

First and foremost you have to explicitly enable pickle support to pickle tracebacks.

Second, you can use tblib just fine without pickle: https://github.com/ionelmc/python-tblib#tblib-traceback-from-dict

@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

I actually have read down to https://github.com/ionelmc/python-tblib#raising where Raising is using pickle in its examples, exclusively. The way I read the docs until that point, pickle seemed the default way of doing things (or even needed) with tblib, not an edge cases that you would only use if and so on. I think I have made my point clear that tblib rather advertises pickle than help warn about it. If we disagree, please feel free to close this ticket.

@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

About the example: the problem is that tblib don't have a to_dict method, thus round-trip testing is done with pickle. It should have a to_dict. Feel free to help here :)

Not sure about what we disagree about? Perhaps the urgency to discourage everyone from using an otherwise powerful serialization library. Tblib only advertises the pickle support, it don't say "pickle is panacea to all life's problems".

Frequent problems with pickle critique, and the article has all of these:

  • No real alternative libraries are being presented, lets just make it hard to use instead. You'd be hard pressed to find a library that's as mature and as fast as pickle.
  • The gravity of the security issue is grotesquely exaggerated and established security practices like signatures are not even being mentioned.
  • Nothing new is presented but it's a hyped subject so it makes views.

@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

I'm unsure what you refer to with signatures in context of pickle. Have a link?

@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

Signatures are checked before unpickling, correct?

@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

Yes.

@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

So they check signatures before unpickling and they have a big warning on pickle and code execution. That's what I would wish for from tblib.

ionelmc added a commit that referenced this issue Mar 8, 2017
@ionelmc
Copy link
Owner

ionelmc commented Mar 8, 2017

Happy now?

@hartwork
Copy link
Author

hartwork commented Mar 8, 2017

😄 Happy would be an exaggeration. I consider that an improvement though. Thank you!

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

No branches or pull requests

2 participants