jupyter / jupyter_client

Jupyter protocol client APIs
https://jupyter-client.readthedocs.io
BSD 3-Clause "New" or "Revised" License
385 stars 285 forks source link

session.py in Jupiter_client seems to be vulnerable to a remote code execution. #292

Open An7i21 opened 7 years ago

An7i21 commented 7 years ago

File: session.py

The following code seems to be parsing third party data received from socket using "deserialize" function which later calls self.unpack in order to handle the received data. "self.unpack" (Depending on the configuration) could set the parser to "Pickle.Unpacker" . The later is known to be prone to remote code execution.

Read more about this issue at the following link: https://www.synopsys.com/blogs/software-security/python-pickling/

Line: 786 def recv(self, socket, mode=zmq.NOBLOCK, content=True, copy=True): 803: msg_list = socket.recv_multipart(mode, copy=copy)
815: return idents, self.deserialize(msg_list, content=content, copy=copy)

Line 108: pickle_unpacker = pickle.loads 321: self.unpack = pickle_unpacker

rgbkrk commented 7 years ago

Running arbitrary code is entirely the point of IPython and Jupyter, which impacts the threat model here. The way in which this session is used is always* to connect to a local interpreter, running as the user who initiated it. Any payload that exploited the pickle format would be wasting energy compared to just running code. Some other caveats:

What's clear to me from this is that Session shouldn't be used with remote kernels (should assume they're unsafe and that you're just passing the messages on). No one uses it for that as far as I know.

An7i21 commented 7 years ago

Any payload that exploited the pickle format would be wasting energy compared to just running code.

You would have been right if the vulnerability was at the server side but In this case the vulnerability exist at the client side. Therefore, a vulnerable configuration (pickle parser) allows the server to execute unauthorized-malicious code on the remote client machine.

No one uses it for that as far as I know.

I understand that no one is currently using the client-server architecture (as far as you know) as separate components (Remote server). However a part of my responsibility is to make sure that no one will accidentally use it like that in the future, or at least know about the risks involved in using it that way.

Carreau commented 7 years ago

Thanks for opening this issue and raising your concern. We appreciate the time and the link you provided about the security concern of pickle, and I believe most of us are aware that pickle can execute arbitrary code.

Side note, in case of potential security vulnerability you can contact security@ipython.org to avoid making the reports public.

The Pickle packer/unpacker are use by ipyparallel between a local kernel and a remotes kernels, in which case both side are already sending each other arbitrary code to execute. So the question is whether pickle is use before we actually check the authenticity of the message or not. And indeed unpack (L926) is called after compare_signature (line 922).

There is a full page on ipyparallel about security. Maybe we should add a section about what we expect is one of the key pieces is taken over an attacker have access to the shared key. But IIRC we expect all clients/engines to be trusted and if one is taken over all can be assumes to be compromised as the attacker have the shared secret and can easily forge request to have any component do anything.

Now, we can put a bigger warning "use pickle only if you know what you are doing, all nodes need to all be trusted and can trigger arbitrary code execution on each other, this is mainly useful for IPyparallel". Seeing that the configurable allow not only pickle, but any user defined unpacker (that is to say you can put exec as an unpacker), i'm not really sure that would be that helpful, or that removing pickle would be more secure. Also you most likely do not want to use pickle, because this requires the same minor version of Python, and all libraries on all sides, and thus why the use case of pickle is used only by ipyparallel.

Would such explanation and bigger warning cover your concerns ?

takluyver commented 7 years ago

I consider the configurable packer an historical anomaly which is kept to support ipyparallel. It's well established now that the Jupyter protocol is json based, and I think using a different serialisation other than in ipyparallel is inviting unnecessary problems. So I'd favour telling users never to use that at all, and maybe working out a deprecation path.

On 10 Sep 2017 10:48 p.m., "Matthias Bussonnier" notifications@github.com wrote:

Thanks for opening this issue and raising your concern. We appreciate the time and the link you provided about the security concern of pickle, and I believe most of us are aware that pickle can execute arbitrary code.

Side note, in case of potential security vulnerability you can contact security@ipython.org to avoid making the reports public.

The Pickle packer/unpacker are use by ipyparallel between a local kernel and a remotes kernels, in which case both side are already sending each other arbitrary code to execute. So the question is whether pickle is use before we actually check the authenticity of the message or not. And indeed unpack (L926) is called after compare_signature (line 922).

There is a full page https://ipyparallel.readthedocs.io/en/latest/security.html on ipyparallel about security. Maybe we should add a section about what we expect is one of the key pieces is taken over an attacker have access to the shared key. But IIRC we expect all clients/engines to be trusted and if one is taken over all can be assumes to be compromised as the attacker have the shared secret and can easily forge request to have any component do anything.

Now, we can put a bigger warning "use pickle only if you know what you are doing, all nodes need to all be trusted and can trigger arbitrary code execution on each other, this is mainly useful for IPyparallel". Seeing that the configurable allow not only pickle, but any user defined unpacker (that is to say you can put exec as an unpacker), i'm not really sure that would be that helpful, or that removing pickle would be more secure. Also you most likely do not want to use pickle, because this requires the same minor version of Python, and all libraries on all sides, and thus why the use case of pickle is used only by ipyparallel.

Would such explanation and bigger warning cover your concerns ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jupyter/jupyter_client/issues/292#issuecomment-328374460, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUA9QA8oEDMECakr7LLFV0_Mhwp2D1Mks5shFlKgaJpZM4PLOzU .

rolweber commented 7 years ago

So would this be an attack vector against the nb2kg demo that connects to a remote kernel gateway via websockets? Or is it limited to cases where the ZMQ connections to a kernel are remote?

rgbkrk commented 7 years ago

Remote ZMQ connections where the payload is pickled.