Skip to content

Commit ecd6844

Browse files
codexByron
andcommitted
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 <sebastian.thiel@icloud.com>
1 parent 5bc2560 commit ecd6844

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

git/config.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,18 @@ def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> s
897897
def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:
898898
if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name):
899899
raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
900+
if label == "section" and isinstance(name, str):
901+
in_quotes = False
902+
escaped = False
903+
for char in name:
904+
if escaped:
905+
escaped = False
906+
elif in_quotes and char == "\\":
907+
escaped = True
908+
elif char == '"':
909+
in_quotes = not in_quotes
910+
elif char == "]" and not in_quotes:
911+
raise ValueError("Git config section names must not contain an unquoted closing bracket")
900912

901913
@needs_values
902914
@set_dirty_and_flush_changes

test/test_config.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,47 @@ def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
194194
self.assertEqual(git_config.get_value("user", "name"), "safe")
195195
self.assertFalse(git_config.has_section("core"))
196196

197+
@with_rw_directory
198+
def test_writer_rejects_unquoted_section_terminators(self, rw_dir):
199+
config_path = osp.join(rw_dir, "config")
200+
bad_sections = ("user] [other", 'submodule "docs"] [other')
201+
safe_section = 'submodule "docs]archive"'
202+
203+
with GitConfigParser(config_path, read_only=False) as git_config:
204+
git_config.add_section("user")
205+
for bad_section in bad_sections:
206+
with pytest.raises(ValueError, match="closing bracket"):
207+
git_config.add_section(bad_section)
208+
with pytest.raises(ValueError, match="closing bracket"):
209+
git_config.set(bad_section, "name", "value")
210+
with pytest.raises(ValueError, match="closing bracket"):
211+
git_config.set_value(bad_section, "name", "value")
212+
with pytest.raises(ValueError, match="closing bracket"):
213+
git_config.add_value(bad_section, "name", "value")
214+
with pytest.raises(ValueError, match="closing bracket"):
215+
git_config.rename_section("user", bad_section)
216+
217+
git_config.set_value("user", "name", "safe")
218+
git_config.set_value(safe_section, "name", "safe")
219+
self.assertEqual(git_config.get_value(safe_section, "name"), "safe")
220+
221+
# A closing bracket inside a quoted subsection name is data, not a section terminator.
222+
with open(config_path, "rb") as config_file:
223+
self.assertIn(
224+
b'[submodule "docs]archive"]\n',
225+
config_file.read(),
226+
"a closing bracket within a quoted subsection name should be preserved",
227+
)
228+
229+
# Reparse the file to verify that rejected names did not inject an [other] section.
230+
with GitConfigParser(config_path, read_only=True) as git_config:
231+
self.assertEqual(
232+
git_config.get_value("user", "name"),
233+
"safe",
234+
"rejected section names corrupted the existing section",
235+
)
236+
self.assertFalse(git_config.has_section("other"), "an unsafe section name injected an [other] section")
237+
197238
@with_rw_directory
198239
def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
199240
config_path = osp.join(rw_dir, "config")

0 commit comments

Comments
 (0)