You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Feature: Remove Pickle usage to improve security of Monai #8527
Is your feature request related to a problem? Please describe.
Pickle is effectively fully functional python code. So loading a python pickle can lead to the complete compromise of the system loading it if an attacker has ever been able to write to it. Often it is assumed that the pickle cannot be accessed, or altered, but since storage mechanisms such as json, and msgpack exist that do not have this vulnerability, moving away from pickle greatly reduces the risk of the application being compromised.
Describe the solution you'd like
I would like for pickle to be removed from all Project-Monai repos in order to set a good example of how to interact with medical images.
In many cases I believe that json and msgpack can work as a solution. msgpack is a non-standard libary but has better support for things such as byte strings.
pickle is not used heavily, and from what I can see it's used in ways to pass data between Monai, so there don't appear to be any external requirements to use it, however if I am wrong, please share it here.
Describe alternatives you've considered
When json and msgpack are not a solution there are still many more solutions that are more data specific such as safetensor. However I don't have enough familiarity with the Monai code base to know which would be applicable. If there is a technical reason that pickle is required and someone shares that in this ticket I will gladly help find a solution.
Hi @Nic-Ma I'll add it to our project for the next release and track it from there. We should look into these solutions mentioned for what is appropriate, safetensors is a possibility for storing large items in places we're using pickles right now such as the caching datasets. Thanks!
@ericspod if you run into anything that doesn't seem to have an alternative feel free to @ me. I've been working on the internal guidance for our teams to resolve these kinds of issues. So if we have a potential solution I can share it, and if we don't then I may still be able to help since I'll probably have to figure it out eventually any way.
Hi @coldwaterq I'm working on a solution now to simply replace use of pickle with torch.save/load in this PR. I realise this still uses pickle but it adds a safety layer on top and suits the purposes that we have for pickling things. The other alternative in my mind was to save tensors/arrays using JSON as special dictionary objects then decode these back later. This is likely to be very slow and creates large files so I'd rather avoid it as well as avoid adding new dependencies to MONAI.
That's a common solution. torch.save/load with weights_only=True as you are using it is a restricted pickler that mitigates the majority of the attacks.
This also makes it clearer what you are saving, such as the couple of safe_globals that had to be added, so it's easier to verify that those don't lead to an attack. And as this field continues to develop it's easier to check if the solutions that completely remove pickle could potentially be applied.
With this increased clearity into what you have to save I think msgpack with safetensor as the backbone for tesnsors could work pretty well here in the future, but the solution you have in that PR solves the known issues, so a more robust solution is just a feature request IMO.
Hi @coldwaterq thanks for looking at the PR. We'll keep iterating on this as things evolve and look into msgpack/safetensors. I do agree these look like good solutions to integrate in the future.
I'm going to close this issue as the upcoming 1.5.1 release will remove pickle usage everywhere except where it is only used store things which are immediately used (auto3dseg, nnunet). These will be revisited at a later time for updating.
Activity
Nic-Ma commentedon Aug 6, 2025
Hi @ericspod , could you please add this ticket to the dev meeting?
Thanks in advance.
ericspod commentedon Aug 6, 2025
Hi @Nic-Ma I'll add it to our project for the next release and track it from there. We should look into these solutions mentioned for what is appropriate, safetensors is a possibility for storing large items in places we're using pickles right now such as the caching datasets. Thanks!
coldwaterq commentedon Aug 7, 2025
@ericspod if you run into anything that doesn't seem to have an alternative feel free to @ me. I've been working on the internal guidance for our teams to resolve these kinds of issues. So if we have a potential solution I can share it, and if we don't then I may still be able to help since I'll probably have to figure it out eventually any way.
ericspod commentedon Sep 15, 2025
Hi @coldwaterq I'm working on a solution now to simply replace use of pickle with
torch.save/loadin this PR. I realise this still uses pickle but it adds a safety layer on top and suits the purposes that we have for pickling things. The other alternative in my mind was to save tensors/arrays using JSON as special dictionary objects then decode these back later. This is likely to be very slow and creates large files so I'd rather avoid it as well as avoid adding new dependencies to MONAI.coldwaterq commentedon Sep 15, 2025
That's a common solution.
torch.save/loadwithweights_only=Trueas you are using it is a restricted pickler that mitigates the majority of the attacks.This also makes it clearer what you are saving, such as the couple of safe_globals that had to be added, so it's easier to verify that those don't lead to an attack. And as this field continues to develop it's easier to check if the solutions that completely remove pickle could potentially be applied.
With this increased clearity into what you have to save I think msgpack with safetensor as the backbone for tesnsors could work pretty well here in the future, but the solution you have in that PR solves the known issues, so a more robust solution is just a feature request IMO.
ericspod commentedon Sep 16, 2025
Hi @coldwaterq thanks for looking at the PR. We'll keep iterating on this as things evolve and look into msgpack/safetensors. I do agree these look like good solutions to integrate in the future.
ericspod commentedon Sep 19, 2025
I'm going to close this issue as the upcoming 1.5.1 release will remove pickle usage everywhere except where it is only used store things which are immediately used (auto3dseg, nnunet). These will be revisited at a later time for updating.