FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #2142 from gitpython-developers/fix-validate-confi… · gitpython-developers/GitPython@d7b029f · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit d7b029f

Browse files
Byron
authored
Merge pull request #2142 from gitpython-developers/fix-validate-config-key-newlines
Validate config key names before writing
2 parents 1085a7c + 5453842 commit d7b029f

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎git/config.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@
7272
See: https://git-scm.com/docs/git-config#_conditional_includes
7373
"""
7474

75+
UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]")
76+
"""Characters that cannot be safely written in config names or values."""
77+
7578

7679
class MetaParserBuilder(abc.ABCMeta): # noqa: B024
7780
"""Utility class wrapping base-class methods into decorators that assure read-only
@@ -778,6 +781,7 @@ def _assure_writable(self, method_name: str) -> None:
778781

779782
def add_section(self, section: "cp._SectionName") -> None:
780783
"""Assures added options will stay in order."""
784+
self._assure_config_name_safe(section, "section")
781785
return super().add_section(section)
782786

783787
@property
@@ -884,10 +888,14 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str:
884888

885889
def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str:
886890
value_str = self._value_to_string(value)
887-
if re.search(r"[\r\n\x00]", value_str):
891+
if UNSAFE_CONFIG_CHARS_RE.search(value_str):
888892
raise ValueError("Git config values must not contain CR, LF, or NUL")
889893
return value_str
890894

895+
def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:
896+
if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name):
897+
raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
898+
891899
@needs_values
892900
@set_dirty_and_flush_changes
893901
def set(
@@ -896,6 +904,8 @@ def set(
896904
option: str,
897905
value: Union[str, bytes, int, float, bool, None] = None,
898906
) -> None:
907+
self._assure_config_name_safe(section, "section")
908+
self._assure_config_name_safe(option, "option")
899909
if value is not None:
900910
value = self._value_to_string_safe(value)
901911
return super().set(section, option, value)
@@ -920,6 +930,8 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo
920930
:return:
921931
This instance
922932
"""
933+
self._assure_config_name_safe(section, "section")
934+
self._assure_config_name_safe(option, "option")
923935
value_str = self._value_to_string_safe(value)
924936
if not self.has_section(section):
925937
self.add_section(section)
@@ -948,6 +960,8 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo
948960
:return:
949961
This instance
950962
"""
963+
self._assure_config_name_safe(section, "section")
964+
self._assure_config_name_safe(option, "option")
951965
value_str = self._value_to_string_safe(value)
952966
if not self.has_section(section):
953967
self.add_section(section)
@@ -968,6 +982,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser":
968982
"""
969983
if not self.has_section(section):
970984
raise ValueError("Source section '%s' doesn't exist" % section)
985+
self._assure_config_name_safe(new_name, "section")
971986
if self.has_section(new_name):
972987
raise ValueError("Destination section '%s' already exists" % new_name)
973988

‎test/test_config.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,37 @@ def test_set_value_rejects_config_injection(self, rw_dir):
163163
self.assertFalse(git_config.has_section("user"))
164164
self.assertFalse(git_config.has_section("core"))
165165

166+
@with_rw_directory
167+
def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
168+
config_path = osp.join(rw_dir, "config")
169+
bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core")
170+
171+
with GitConfigParser(config_path, read_only=False) as git_config:
172+
git_config.add_section("user")
173+
for bad_key in bad_keys:
174+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
175+
git_config.add_section(bad_key)
176+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
177+
git_config.set(bad_key, "hooksPath", "/tmp/hooks")
178+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
179+
git_config.set("user", bad_key, "/tmp/hooks")
180+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
181+
git_config.set_value(bad_key, "hooksPath", "/tmp/hooks")
182+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
183+
git_config.set_value("user", bad_key, "/tmp/hooks")
184+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
185+
git_config.add_value(bad_key, "hooksPath", "/tmp/hooks")
186+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
187+
git_config.add_value("user", bad_key, "/tmp/hooks")
188+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
189+
git_config.rename_section("user", bad_key)
190+
191+
git_config.set_value("user", "name", "safe")
192+
193+
with GitConfigParser(config_path, read_only=True) as git_config:
194+
self.assertEqual(git_config.get_value("user", "name"), "safe")
195+
self.assertFalse(git_config.has_section("core"))
196+
166197
@with_rw_directory
167198
def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
168199
config_path = osp.join(rw_dir, "config")

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL