-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create rule S6985 : Usage of "torch.load" can lead to untrusted code execution #3976
Conversation
9e2685a
to
a454852
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a minor suggestion. And don't forget to add a title to the PR.
rules/S6985/python/rule.adoc
Outdated
If the model comes from an untrusted source, an attacker could inject a malicious payload which would be executed during the deserialization. | ||
|
||
== How to fix it | ||
Use a safer alternative to load the model, such as `safetensors.torch.load_model`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you will need to add a new line before the title. === Code examples
|
||
== Why is this an issue? | ||
|
||
Under the hood, `torch.load` uses the `pickle` library to load the model and the weights. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could add just a small introduction line. In PyTorch it is common practice to load a serialized model
or something like that.
Quality Gate passed for 'rspec-frontend'Issues Measures |
Quality Gate passed for 'rspec-tools'Issues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a small comment, otherwise it looks good!
import safetensors | ||
|
||
model = MyModel() | ||
safetensors.torch.load_model(model, 'model.pth') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I would only provide the code that would fix the issue in the Noncompliant
section. So I would remove the line 36, from this example. And if you want to showcase it, I would create a different code example, but frankly I do not think it is worth it here.
479a4be
to
b9481e5
Compare
Quality Gate passed for 'rspec-tools'Issues Measures |
Quality Gate passed for 'rspec-frontend'Issues Measures |
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: