Merge pull request #2217 from gitpython-developers/fix-advisory

Require explicit opt-in for filesystem diffs

Author: Byron

doc/source/changes.rst

@@ -8,6 +8,7 @@ Changelog
 Security fixes for
 
 * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-g5vv-9gxw-82hx
+* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-whh4-5q6c-9v3x
 
 If you can, also try and provide feedback on the upcoming v4 branch
 https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.

git/diff.py

@@ -239,8 +239,8 @@ def diff(
             to be read and diffed.
 
         :param allow_unsafe_options:
-            If ``True``, allow options such as ``--output`` and ``-O`` that can write to
-            or read from arbitrary filesystem paths.
+            If ``True``, allow options such as ``--output``, ``--no-index``, and ``-O``
+            that can write to or read from arbitrary filesystem paths.
 
         :param kwargs:
             Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to

git/repo/base.py

@@ -209,6 +209,8 @@ class Repo:
     ]
 
     unsafe_git_diff_options = unsafe_git_revision_options + [
+        # Treats path operands as arbitrary filesystem paths.
+        "--no-index",
         # Reads caller-controlled order patterns from an arbitrary file.
         "-O",
         "--orderfile",

test/test_diff.py

@@ -412,6 +412,17 @@ def test_diff_rejects_unsafe_output_options(self):
         commit.diff(output=allowed_target, allow_unsafe_options=True)
         self.assertTrue(osp.isfile(allowed_target))
 
+    def test_diff_rejects_no_index(self):
+        calls = (
+            lambda: self.rorepo.head.commit.diff(no_index=True),
+            lambda: self.rorepo.head.commit.diff(other="--no-index"),
+            lambda: self.rorepo.index.diff(None, no_index=True),
+            lambda: self.rorepo.index.diff("--no-index"),
+        )
+        for call in calls:
+            with self.assertRaises(UnsafeOptionError):
+                call()
+
     def test_diff_interface(self):
         """Test a few variations of the main diff routine."""
         assertion_map = {}