Merge pull request #2176 from gitpython-developers/fix-config-injection

fix: validate config section delimiters

Author: Byron

git/config.py

@@ -897,6 +897,22 @@ def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> s
     def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:
         if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name):
             raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
+        if label == "section" and isinstance(name, str):
+            in_quotes = False
+            escaped = False
+            for index, char in enumerate(name):
+                if escaped:
+                    escaped = False
+                elif in_quotes and char == "\\":
+                    escaped = True
+                elif char == '"':
+                    if not in_quotes and (index == 0 or name[index - 1] not in " \t"):
+                        raise ValueError("Git config quoted subsection names must begin after whitespace")
+                    in_quotes = not in_quotes
+                elif char == "]" and not in_quotes:
+                    raise ValueError("Git config section names must not contain an unquoted closing bracket")
+            if in_quotes:
+                raise ValueError("Git config section names must not contain an unterminated quote")
 
     @needs_values
     @set_dirty_and_flush_changes

test/test_config.py

@@ -194,6 +194,53 @@ def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
             self.assertEqual(git_config.get_value("user", "name"), "safe")
             self.assertFalse(git_config.has_section("core"))
 
+    @with_rw_directory
+    def test_writer_rejects_unquoted_section_terminators(self, rw_dir):
+        config_path = osp.join(rw_dir, "config")
+        bad_sections = (
+            "user] [other",
+            'user"] [other"',
+            'submodule "docs"] [other',
+            'submodule "docs] [other',
+            'submodule "docs] [other\\',
+        )
+        safe_section = 'submodule "docs]archive"'
+
+        with GitConfigParser(config_path, read_only=False) as git_config:
+            git_config.add_section("user")
+            for bad_section in bad_sections:
+                with pytest.raises(ValueError, match="section name"):
+                    git_config.add_section(bad_section)
+                with pytest.raises(ValueError, match="section name"):
+                    git_config.set(bad_section, "name", "value")
+                with pytest.raises(ValueError, match="section name"):
+                    git_config.set_value(bad_section, "name", "value")
+                with pytest.raises(ValueError, match="section name"):
+                    git_config.add_value(bad_section, "name", "value")
+                with pytest.raises(ValueError, match="section name"):
+                    git_config.rename_section("user", bad_section)
+
+            git_config.set_value("user", "name", "safe")
+            git_config.set_value(safe_section, "name", "safe")
+            self.assertEqual(git_config.get_value(safe_section, "name"), "safe")
+
+        # A closing bracket inside a quoted subsection name is data, not a section terminator.
+        with open(config_path, "rb") as config_file:
+            self.assertIn(
+                b'[submodule "docs]archive"]\n',
+                config_file.read(),
+                "a closing bracket within a quoted subsection name should be preserved",
+            )
+
+        # Reparse the file to verify that rejected names did not inject an [other] section.
+        with GitConfigParser(config_path, read_only=True) as git_config:
+            self.assertEqual(
+                git_config.get_value("user", "name"),
+                "safe",
+                "rejected section names corrupted the existing section",
+            )
+            self.assertFalse(git_config.has_section("other"), "an unsafe section name injected an [other] section")
+
     @with_rw_directory
     def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
         config_path = osp.join(rw_dir, "config")