Skip to content

Commit 1ed1b92

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 1ed1b92

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

git/config.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,22 @@ 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 index, char in enumerate(name):
904+
if escaped:
905+
escaped = False
906+
elif in_quotes and char == "\\":
907+
escaped = True
908+
elif char == '"':
909+
if not in_quotes and (index == 0 or name[index - 1] not in " \t"):
910+
raise ValueError("Git config quoted subsection names must begin after whitespace")
911+
in_quotes = not in_quotes
912+
elif char == "]" and not in_quotes:
913+
raise ValueError("Git config section names must not contain an unquoted closing bracket")
914+
if in_quotes:
915+
raise ValueError("Git config section names must not contain an unterminated quote")
900916

901917
@needs_values
902918
@set_dirty_and_flush_changes

test/test_config.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,53 @@ 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 = (
201+
"user] [other",
202+
'user"] [other"',
203+
'submodule "docs"] [other',
204+
'submodule "docs] [other',
205+
'submodule "docs] [other\\',
206+
)
207+
safe_section = 'submodule "docs]archive"'
208+
209+
with GitConfigParser(config_path, read_only=False) as git_config:
210+
git_config.add_section("user")
211+
for bad_section in bad_sections:
212+
with pytest.raises(ValueError, match="section name"):
213+
git_config.add_section(bad_section)
214+
with pytest.raises(ValueError, match="section name"):
215+
git_config.set(bad_section, "name", "value")
216+
with pytest.raises(ValueError, match="section name"):
217+
git_config.set_value(bad_section, "name", "value")
218+
with pytest.raises(ValueError, match="section name"):
219+
git_config.add_value(bad_section, "name", "value")
220+
with pytest.raises(ValueError, match="section name"):
221+
git_config.rename_section("user", bad_section)
222+
223+
git_config.set_value("user", "name", "safe")
224+
git_config.set_value(safe_section, "name", "safe")
225+
self.assertEqual(git_config.get_value(safe_section, "name"), "safe")
226+
227+
# A closing bracket inside a quoted subsection name is data, not a section terminator.
228+
with open(config_path, "rb") as config_file:
229+
self.assertIn(
230+
b'[submodule "docs]archive"]\n',
231+
config_file.read(),
232+
"a closing bracket within a quoted subsection name should be preserved",
233+
)
234+
235+
# Reparse the file to verify that rejected names did not inject an [other] section.
236+
with GitConfigParser(config_path, read_only=True) as git_config:
237+
self.assertEqual(
238+
git_config.get_value("user", "name"),
239+
"safe",
240+
"rejected section names corrupted the existing section",
241+
)
242+
self.assertFalse(git_config.has_section("other"), "an unsafe section name injected an [other] section")
243+
197244
@with_rw_directory
198245
def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
199246
config_path = osp.join(rw_dir, "config")

0 commit comments

Comments
 (0)