Add security warning about the from_pretrained() method
Author: lewtun
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

This PR adds a warning to the docs about the `from_pretrained()` method being susceptible to malicious code injection. This is similar to other warnings provided in the [PyTorch](https://pytorch.org/docs/stable/generated/torch.load.html#torch.load) and [Python](https://docs.python.org/3/library/pickle.html#module-pickle) docs.

For now I've put this in the autoclass tutorial, but can also put it in the API docs if we agree this makes sense.

Questions:

* Does the same issue apply to TensorFlow models?
* Does the [malware scanner](https://huggingface.co/docs/hub/security-malware) catch malicious code injection for _all_ Hub repos (public and private)?

h/t to @yk who pointed this out to me.

## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @LysandreJik

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->

cc @sgugger @McPatate 


Comments:
> Does the [malware scanner](https://huggingface.co/docs/hub/security-malware) catch malicious code injection for all Hub repos (public and private)?

It doesn't "catch malicious code injection" per se, it extracts the list of module-function pairs that can be called when unpickling. We still haven't implemented anything on top of that.

To answer your question, we're only scanning public repositories atm.

_The documentation is not available anymore as the PR was closed or merged._

Should a mention of the malware scanner also be added to the sentence? Right now it reads like using the Hub is a security issue whereas it's really `torch.load` that has a security issue; and the Hub enables verifying that the code is malware-free with both the malware scanner and signed commits.

I'd rather focus on showing how using the Hub is likely a smaller security risk than using an external tool like GDrive where no such verification can be done.

> Should a mention of the malware scanner also be added to the sentence? Right now it reads like using the Hub is a security issue whereas it's really `torch.load` that has a security issue; and the Hub enables verifying that the code is malware-free with both the malware scanner and signed commits.
> 
> I'd rather focus on showing how using the Hub is likely a smaller security risk than using an external tool like GDrive where no such verification can be done.

Good idea! Added a sentence about the scanner and reworded the text in [8098c4b](https://github.com/huggingface/transformers/pull/18801/commits/8098c4bede79f2e3f95308fbd47fd5b326f502e3)

I'm not disagreeing that the HF hub is a smaller risk compared to other things, just pointing out that the malware scanner currently lists my model as safe (and due to the mechanics of torch.save, it will always be evadable). And signed commits are super useful, but they don't mitigate this particular problem, as the signatory would be me, and therefore the signature valid.

> due to the mechanics of torch.save, it will always be evadable

@yk would you mind expanding on that ?

> @yk would you mind expanding on that ?

torch.save uses pickle, which in turn allows for arbitrary code execution. If the malware scanner detects loaded modules, I can simply un-load them after I've used them. If the malware scanner looks for the presence of certain instructions, strings, etc. I can always evade it somehow, that's just the nature of turing-completeness. I could probably even DDOS the malware scanner by just running an infinite loop. I'm happy to show more, and I'm going to, but I was waiting to release that info publicly before going through the responsible disclosure process here. DM me in case you want an early insight.

> If the malware scanner detects loaded modules, I can simply un-load them after I've used them.

I get the list of module-function pairs directly from the pickle opcodes ([`pickletools.genops`](https://docs.python.org/3/library/pickletools.html#pickletools.genops)), without executing anything. We went through some pretty [sophisticated exploits](https://ctftime.org/writeup/16723) and from what I saw when replicating there is always a trace of the module/function, e.g. if you wanted to run `eval` but alias it, you would still see the original `eval` reference.

> I could probably even DDOS the malware scanner by just running an infinite loop.

Since I'm going through the instruction list in the pickle file, there would be no DDOS possible via code execution. You could always generate a massive pickle file to bloat the scanner, but then you'd run into issues like uploading files to the hub before our scanner even goes through them + we can easily add heuristics to mark these files as inherently unsafe.

> If the malware scanner looks for the presence of certain instructions, strings, etc. I can always evade it somehow, that's just the nature of turing-completeness.

I'd like to see how you do that, happy to have the early insight, but I can wait for the public release :)

Note that you cannot serialize functions or lambdas in a pickle file, you can only execute functions that are in scope at deserialization time.

cc @adrinjalali

EDIT: thank you for expanding !

It seems there are actually ways to DDOS the scanner, we currently wouldn't catch the sploits [referenced here](https://github.com/moreati/pickle-fuzz#denial-of-service) (meaning we'd proceed to deserialization).

> I get the list of module-function pairs directly from the pickle opcodes

fair point, then I interpreted your previous statement in the wrong way. yea that could actually work to mitigate some of these things. At this time, my model here is still marked as safe, though: https://huggingface.co/ykilcher/totally-harmless-model/tree/main

> At this time, my model here is still marked as safe, though: https://huggingface.co/ykilcher/totally-harmless-model/tree/main

That's normal, at the time we haven't implemented any checks per se, we just extract the data from the pickles on the hub.

(coming soon)

> Should a mention of the malware scanner also be added to the sentence? Right now it reads like using the Hub is a security issue whereas it's really torch.load that has a security issue; and the Hub enables verifying that the code is malware-free with both the malware scanner and signed commits.

@LysandreJik saving and loading pickle files is only insecure if the author is not _trusted_, which is the case for the HF hub. The hub is a place where people share arbitrary pickle files, so it becomes a hub issue. It wouldn't be an issue if people kept using pickles from sources they trust. And the scanner doesn't really detect all vulnerabilities, and can't. For instance, when joblib does an `eval` on a given `n_jobs` parameter, there you could do whatever you want. In terms of signatures, we check signatures, but we don't check who the people behind the accounts are (and I don't think we should).

So I do think people should be wary when they load pickles from the hub.

Note that AFAIK in pretty much all major communities (like pytorch, sklearn, etc) people know about this and are working on having better solutions. But for now, users should be aware of the risks.

BTW @McPatate as discussed it would be awesome if we could write a post (or even some documentation) about what we currently know about pickle safety, and the potential next steps we are working on.

And maybe @yk you'd be interested in taking a look? Would be awesome to have your insights :)

@julien-c sure, I'm happy to contribute what I know

hi @yk the team (@McPatate in particular) wrote this document https://huggingface.co/docs/hub/security-pickle the doc's source is at https://github.com/huggingface/hub-docs/pull/294 – would love to get your feedback (including on the proposed solutions) Thanks!