torch.jit.annotations.parse_type_line is not safe (command injection) even it seems already deprecated.

[Lyutoon]

🐛 Describe the bug

In torch.jit.annotations, it looks like there are some functions that are deprecated, but still retain code, which may lead to some backdoors, especially since some of these functions still use eval while implementing.
But now I'm not sure if there are some features (jit decorator) in some version of pytorch are still using this function parse_type_line or get_signature which calls parse_type_line, if so, it can cause RCE, if not, maybe someone can also leave a backdoor by calling this function while writing code and share it to the people.

import torch

torch.jit.annotations.parse_type_line('# type: __import__("os").system("ls") -> 234', None, 1)

Versions

master

cc @ezyang @gchanan @zou3519 @EikanWang @jgong5 @wenzhe-nrv @sanchitintel

[malfet]

Marking for triage review(and not assigning oncall: jit yet otherwise it will disappear to the void) to discuss what to do with those kinds of security issues, which, in my opinion, is pretty minor: if one have an access to local Python runtime they can do anything they want.

[Lyutoon]

Marking for triage review(and not assigning oncall: jit yet otherwise it will disappear to the void) to discuss what to do with those kinds of security issues, which is in my opinion is pretty minor: if one have an access to local Python runtime they can do anything they want.

Yes, this seems not a very urgent bug, but we still need to pay attention to these dangerouse functions such as eval. And in CVE-2022-0845, this bug is also caused by using eval to parse the args so it leads to code injection, and it seems also must have an access to local python. To be honest, I don't know how people think about these kind of problems, but we need to pay more attention :p. So we need a discuss about it whether it can be considered as a big security problem.

[malfet]

We should have a doc marking unsafe function and safe versions of the same.
And also, probably should not use eval, if possible.

[Lyutoon]

We should have a doc marking unsafe function and safe versions of the same. And also, probably should not use eval, if possible.

That's right, there was also a cve (https://github.com/tensorflow/tensorflow/blob/master/tensorflow/security/advisory/tfsa-2022-060.md) in tensorflow that used eval in saved_model_cli and caused code injection. Also I've found that PaddlePaddle has also eval problems (https://github.com/PaddlePaddle/Paddle/blob/develop/security/advisory/pdsa-2022-002.md). But sometimes, if we do not use eval, the code will become much more complex. Maybe developers can just add a critical check about the parameters of the function to avoid this problem easily (if possible).

[malfet]

It seems reasonable, that valid type annotations should not have any funciton/method calls, so splitting it into compile (which is safe)->error on function calls->eval would secure the deprecated codebase without overcomplicating deprecated code too much

[roywei]

Hi guys, is there any plan to release this patch to torch 1.12.x and 1.13.x? any ETAs? thanks!

[seemethere]

Hi guys, is there any plan to release this patch to torch 1.12.x and 1.13.x? any ETAs? thanks!

We don't currently have any plans to do continued releases for 1.12.x but given the security implications of this this patch will be included in the next patch release for 1.13.x

[bendeaton]

Can someone explain in more detail what the actual vulnerability is and how to determine whether a given code base actually uses the offending code?

The Pytorch/jit docs (link) note that “Python 3 type hints can be used in place of torch.jit.annotate” ; does this imply that it’s possible to ameliorate the vulnerability by fully type-hinting all user functions compiled by jit, and if so how can these functions be identified?

Does this vulnerability apply only to compiled functions using Pytorch, or does it extend to things that could be injected into an inference request to trigger the privilege escalation issue?

[ezyang]

I believe the vulnerability is, if someone crafts a malicious Python file, and then you compile it TorchScript, it can trigger arbitrary code execution. That being said, I'm not really sure what your threat model is, since you probably already have problems if you're compiling arbitrary malicious Python code with TorchScript?

[ardell]

That was my assessment as well, based on a cursory read of the issue and the docs I could find.

It does seem confusing that the CVSS score lists the attack vector as "Network", attack complexity as "Low", and privileges required as "None" if this vulnerability requires compiling the malicious python code to TorchScript.

The CVSS makes it seem like someone could trigger the vulnerability via (for example) a maliciously-crafted inference request (without any arbitrary code compilation) -- which would certainly be a critical security vulnerability. I'm interested in ruling that out if possible, especially while we wait for a patched version to be released...

[ezyang]

Definite not via user controlled tensor inputs. There is no vector for type annotations there

[malfet]

I guess the problem here is that torch.jit.annotations.parse_type_line looks like a public API, which means that dev can use it in their code to extract function annotation and arbitrary code execution feels like a very unexpected side-effect of calling such function.

[abustamante-coveo]

hello, is there an ETA on 1.13.1?

[malfet]

@abustamante-coveo tentatively Dec 15th, see #89855

[abustamante-coveo]

hello, Snyk still marks torch as having a critical vulnerability even after the 1.13.1 patch. Was this vulnerability supposed to be patched on that version?

[vikcher]

hello, Snyk still marks torch as having a critical vulnerability even after the 1.13.1 patch. Was this vulnerability supposed to be patched on that version?

Looks like their vulnerability database is not updated after the fix. It shows that the fix is pushed but not published:
https://security.snyk.io/vuln/SNYK-PYTHON-TORCH-3149871