diff --git a/git/config.py b/git/config.py index 97ae054e5..82747eadd 100644 --- a/git/config.py +++ b/git/config.py @@ -72,6 +72,9 @@ See: https://git-scm.com/docs/git-config#_conditional_includes """ +UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]") +"""Characters that cannot be safely written in config names or values.""" + class MetaParserBuilder(abc.ABCMeta): # noqa: B024 """Utility class wrapping base-class methods into decorators that assure read-only @@ -778,6 +781,7 @@ def _assure_writable(self, method_name: str) -> None: def add_section(self, section: "cp._SectionName") -> None: """Assures added options will stay in order.""" + self._assure_config_name_safe(section, "section") return super().add_section(section) @property @@ -884,10 +888,14 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str: def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str: value_str = self._value_to_string(value) - if re.search(r"[\r\n\x00]", value_str): + if UNSAFE_CONFIG_CHARS_RE.search(value_str): raise ValueError("Git config values must not contain CR, LF, or NUL") return value_str + 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) + @needs_values @set_dirty_and_flush_changes def set( @@ -896,6 +904,8 @@ def set( option: str, value: Union[str, bytes, int, float, bool, None] = None, ) -> None: + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") if value is not None: value = self._value_to_string_safe(value) return super().set(section, option, value) @@ -920,6 +930,8 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -948,6 +960,8 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -968,6 +982,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser": """ if not self.has_section(section): raise ValueError("Source section '%s' doesn't exist" % section) + self._assure_config_name_safe(new_name, "section") if self.has_section(new_name): raise ValueError("Destination section '%s' already exists" % new_name) diff --git a/test/test_config.py b/test/test_config.py index a9dcdb087..3ddaf0a4b 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -163,6 +163,37 @@ def test_set_value_rejects_config_injection(self, rw_dir): self.assertFalse(git_config.has_section("user")) self.assertFalse(git_config.has_section("core")) + @with_rw_directory + def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir): + config_path = osp.join(rw_dir, "config") + bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core") + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_key in bad_keys: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_section(bad_key) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.rename_section("user", bad_key) + + git_config.set_value("user", "name", "safe") + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual(git_config.get_value("user", "name"), "safe") + self.assertFalse(git_config.has_section("core")) + @with_rw_directory def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir): config_path = osp.join(rw_dir, "config")