CVE-2024-11392/11393/11394 vulnerabilities
Author: JWYang0329
Pull request: no
Labels: name: Feature request

### Feature request

https://vuldb.com/?id.285443
https://vuldb.com/?id.285442
https://vuldb.com/?id.285441
Have these vulnerabilities been fixed in the current version, or are there any plans to fix them in the near future?
And what is the scope of impact of these vulnerabilities?

### Motivation

confirm the scope of impact of these vulnerabilities and the fix version

### Your contribution

no

Comments:
These vulnerabilities generally stem from specific model class conversion scripts in the `transformers` codebase. These conversion scripts are not imported or called by the main library, and are only included as a convenience for users who want to convert models from the original format they were released in to HF format. Because the original models are often released as zip files, non-safetensors torch weights, etc, the conversion scripts must open those formats, which results in a potential vulnerability as these formats can contain malicious payloads.

However, to exploit the vulnerability, an attacker would have to craft a malicious model and then induce the user to call a specific obscure conversion script on it, which we don't consider to be a realistic attack vector. Since these vulnerabilities exist only in those accessory conversion scripts, and not in core library functions, and because there's no real way to mitigate them without deleting the scripts, we basically just ignore them!

@Rocketknight1 Get! Thanks for your detailed explanation!

No probs! Closing for now since this seems resolved, but feel free to reopen or follow up if needed.

@Rocketknight1 

1. w.r.t `specific model class conversion scripts in the transformers codebase` could you pls. provide the script names in the transformers codebase.. In our org, we can't go ahead with latest transformers package version owing to these vulnerabilities unless we specifically delete these from our docker images.. downgrading to older versions is not an option since there are more vulnerabilities shown up moving to an older version in our scans
3. So, latest versions, there are no plans of removing these scripts from transformers codebase?

@mithunvb These scripts are always contained in `src/transformers/models/[model]/`, and (I think) always follow the naming pattern `convert_*.py`. For example, `src/transformers/models/bert/convert_bert_original_tf2_checkpoint_to_pytorch.py`.

I believe removing all scripts following the pattern `src/transformers/models/**/convert*.py` should be sufficient, but if this is critical you may wish to check the model dirs individually for Python conversion scripts, in case there's one with a different naming scheme. You do not need to check any dirs outside of `src/transformers/models`.

Thanks @Rocketknight1 for the quick reply :) 

I find 235 files with this pattern `models/**/convert*.py`

So, latest versions, there are no plans of removing these scripts from transformers codebase? since we keep updating the versions, every time removing these files is not a neat option.. If the vulnerability is there, guess these should be removed from transformers in upcoming versions. Thanks

I also work at a company that is very serious about not using vulnerable versions of packages and is giving me trouble about these. If these truly are rarely used utility scripts, maybe they could be shuffled off to a separate distributable, or otherwise be an optional install? As long as you are tagged with these CVEs you may have problems getting used at many bigger companies.

@mithunvb @jesnie your suggestions are noted - I'll raise this internally and let you know if we decide anything!

Thanks for considering/raising the request. I was just coming here to say the same thing, in case it adds additional weight to the request. Our software is deployed at customer sites who follow standards such as PCI DSS. We've already made the change to remove the convert python files, but the CVEs still show in the results of a security scan (AWS Inspector) of our docker images. Our customer(s) may be able to grant an except, but it's a complication and much simpler if the issue can be avoided altogether.

@Rocketknight1 So to sum things up:
1. Can you confirm, whether PyPI transformers is affected by those CVEs or not?
2. If its affected, is there an estimated upcoming fix?

Thank you!

@noren95 @JSandler right now the conversion scripts are included in the pypi wheels. After some internal discussion, we think we might keep the conversion scripts in the repo but exclude them from pypi wheels in future. Would this be sufficient, or would it still cause vulnerability scanner issues for you?

I'm pretty sure we should be fine, as long as the scripts aren't distributed in the wheels.

cc @jesnie @noren95 @JSandler @mithunvb we have opened PR #35296 to address this

@Rocketknight1 Thanks so much! Does it resolve all 3 CVEs?

It should! None of the affected code will be present in release branches / release wheels in future once the PR is merged. However, I'm not sure how exactly vulnerability databases handle this. The conversion scripts will still be present on the `main` branch, and in older release branches/wheels. Hopefully we can get them to stop flagging us, especially since these aren't serious vulnerabilities to begin with!

PR is merged - closing for now, but feel free to ping me here if there are further compliance issues. We don't want this to become a major issue for `transformers` users!

@Rocketknight1 thanks for the fast turnaround on this. I imagine it will take some time to update the vulnerability databases but hopefully the tools catch up quick.

> It should! None of the affected code will be present in release branches / release wheels in future once the PR is merged. However, I'm not sure how exactly vulnerability databases handle this. The conversion scripts will still be present on the `main` branch, and in older release branches/wheels. Hopefully we can get them to stop flagging us, especially since these aren't serious vulnerabilities to begin with!

Thanks @Rocketknight1 for the quick fix.. I can't talk for all kinds of vulnerability scans out there but since its mostly used in production via pypi installs, not having them as part of the install should solve majority of the cases I would assume unless someone specifically using it via cloning and come across this in their scans raise an issue here..

That's what we're hoping! The first version without the conversion scripts should be **v4.48**.

If anyone is still experiencing issues with vulnerability scanners or compliance problems after that version, feel free to ping me here or open a new issue. We may reopen this if removing the conversion scripts from release wheels doesn't resolve the problem for everyone.

> Thanks for considering/raising the request. I was just coming here to say the same thing, in case it adds additional weight to the request. Our software is deployed at customer sites who follow standards such as PCI DSS. We've already made the change to remove the convert python files, but the CVEs still show in the results of a security scan (AWS Inspector) of our docker images. Our customer(s) may be able to grant an except, but it's a complication and much simpler if the issue can be avoided altogether.

+1. Thank you @Rocketknight1 for the fix! Much easier than trying to negotiate with the security side

> That's what we're hoping! The first version without the conversion scripts should be **v4.48**.
> 
> If anyone is still experiencing issues with vulnerability scanners or compliance problems after that version, feel free to ping me here or open a new issue. We may reopen this if removing the conversion scripts from release wheels doesn't resolve the problem for everyone.

@Rocketknight1 when is 4.48 expected to be available on pypi? Thanks

Hi @mithunvb I don't have an exact release date, but probably within the next 2 weeks. We're overdue for the next release of `transformers`, but timelines were pushed back because of the Christmas period

Thanks @Rocketknight1 .. The reason I asked was we are having very difficult exception approval process for every change in the code because of these vulnerabilities( we have removed the files but the scan will report based on pypi release version ).. anyway, if its around 2 weeks, then its ok.. Thanks for the fix

Hi @JWYang0329 @mithunvb @jesnie @noren95 @elshize, version 4.48 was released today and conversion scripts were excluded. I believe the release branch and release wheels should be vulnerability-free, but it might take some time to update vulnerability scanners etc.

Thanks @Rocketknight1 :) 

Hey, just reporting back. We deployed the new version recently and Mend is now not detecting any vulnerabilities 🎉 

Thank you @Rocketknight1 for helping out with this.