Skip to content

Feature: Remove Pickle usage to improve security of Monai #8527

Closed

Description

@coldwaterq

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.

Additional context
https://docs.python.org/3/library/pickle.html - Official Python docs state "Warning The pickle module is not secure"

Activity

  1. Nic-Ma commented on Aug 6, 2025

    @Nic-Ma
    Contributor

    Hi @ericspod , could you please add this ticket to the dev meeting?

    Thanks in advance.

  2. moved this to Todo in MONAI v1.6on Aug 6, 2025
  3. ericspod commented on Aug 6, 2025

    @ericspod
    Member

    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!

  4. coldwaterq commented on Aug 7, 2025

    @coldwaterq
    Author

    @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.

  5. ericspod commented on Sep 15, 2025

    @ericspod
    Member

    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.

  6. coldwaterq commented on Sep 15, 2025

    @coldwaterq
    Author

    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.

  7. ericspod commented on Sep 16, 2025

    @ericspod
    Member

    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.

  8. ericspod commented on Sep 19, 2025

    @ericspod
    Member

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions