From 1ed1b924f4e2d2ee7bab296df77b978af21853f1 Mon Sep 17 00:00:00 2001 From: "GPT 5.6" Date: Mon, 20 Jul 2026 14:13:22 +0200 Subject: [PATCH] fix: validate config section delimiters GHSA-3rp5-jjmw-4wv2 identified that configuration section names could alter the structure of serialized config despite the existing control-character checks. Reject unquoted closing section delimiters across all writer entry points while preserving valid delimiters inside quoted subsections and the existing option-name behavior. Add regression coverage for unsafe plain and quoted-subsection-shaped names as well as a valid quoted subsection. Git baseline: a23bace963d508bd96983cc637131392d3face18. Git config parsing treats an unquoted closing bracket as the end of a basic section header, while its extended form permits brackets inside a quoted subsection and requires a final bracket after the closing quote. Co-authored-by: Sebastian Thiel --- git/config.py | 16 +++++++++++++++ test/test_config.py | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/git/config.py b/git/config.py index 64f501424..821710ea3 100644 --- a/git/config.py +++ b/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 diff --git a/test/test_config.py b/test/test_config.py index c4e39db48..361a51fa9 100644 --- a/test/test_config.py +++ b/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")