-
-
Notifications
You must be signed in to change notification settings - Fork 954
USE_SHELL and command injection #1896
-
It has been decided to publicly disclose specific details of how an application that sets Git.USE_SHELL
to True
can become vulnerable, because this information is already widely known by security researchers yet may not always be obvious to application developers.
The Git.USE_SHELL
class attribute has the default value of False
. Setting it to True
makes applications vulnerable to OS command injection, if untrusted text ever makes its way into any part of any argument that GitPython passes to any git command.
- This is hard to prevent except by refraining from setting
USE_SHELL
toTrue
, and it applies even to text used as paths passed to a command after--
, and in other cases where it may be unintuitive. - It happens both when running git commands explicitly through dynamic attributes of a
Git
object, and implicitly through functionality provided byRepo
and other classes.
For this reason, setting USE_SHELL
to True
should be avoided. Although the USE_SHELL
attribute is retained for backward compatibility because removing it would be a breaking change, it has been deprecated. Its docstring has been updated to warn about this, though specific details of the risks are not given there, and are instead presented below.
It is likewise unsafe to pass shell=True
to any GitPython function that accepts it, but in the case of USE_SHELL
, the effect is much broader, affecting all function calls where shell
could have been passed but was not.
Although Git.USE_SHELL
is available on all platforms, it is in practice unlikely to be set to True
except on Windows. Setting it to True
had at one time been recommended to work around a Windows-related bug that was fixed properly in GitPython 2.0.8. Furthermore, most GitPython functionality visibly breaks if it is set to True
on other systems, as it is only on Windows that subprocess.Popen
accepts a sequence of separate arguments (rather than a single string) as a command even when shell=True
.
Therefore, examples of how it can be exploited are presented here only for the Windows cmd.exe
shell. These examples are not exhaustive. When USE_SHELL
is set to True
...
-
This creates or truncates
outfile
, which can be any path that the user running the application has write access to:g = Git() g.diff("HEAD>outfile")
Other redirection operators, including
<
to read files and>>
to append to them, can also be used. -
This runs the Windows calculator program (a useful choice because it is easy to observe), and can be trivially modified to run any command that can be expressed without spaces:
g = Git() g.diff("HEAD&&calc")
That spaces may induce quoting in the
Popen
call that, while not intended to suppress special shell syntax, sometimes does so, is not a significant mitigation. Even if that cannot be overcome, redirection can be used to pass quoted commands to a scripting runtime, or parts of a command can be passed through as (all or part of) multiple arguments. -
This likewise runs the Windows calculator program or, when modified, any program:
r = Repo() r.iter_commits("||calc")
Either the
&&
and||
operators can be used on dynamicGit
attributes, calls toRepo
methods, and elsewhere, and can be combined with arbitrary redirection. -
The
^
character is acmd.exe
escape character, as well as being syntactically significant in Git commands. This is likely to cause problems by accident, and may also aid in exploitation. An example of an accidental malfunction due to it is:g = Git() g.diff("HEAD^", "HEAD")
This returns an empty string, even outside the rare case that the
HEAD
commit makes no changes. Its effect is the same asg.diff("HEAD", "HEAD")
.
As mentioned above, these examples not exhaustive, and there are other forms of shell syntax that can be injected.
These examples, and the risk they represent, apply only when GitPython is made to run commands through a shell, which USE_SHELL
does. In the absence of a True
value for USE_SHELL
or the optional shell
argument to functions that accept it, this risk of OS command injection is absent.
Beta Was this translation helpful? Give feedback.