diff --git a/git/config.py b/git/config.py index c6eaf8f7b..97ae054e5 100644 --- a/git/config.py +++ b/git/config.py @@ -882,6 +882,24 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str: return str(value) return force_text(value) + 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): + raise ValueError("Git config values must not contain CR, LF, or NUL") + return value_str + + @needs_values + @set_dirty_and_flush_changes + def set( + self, + section: str, + option: str, + value: Union[str, bytes, int, float, bool, None] = None, + ) -> None: + if value is not None: + value = self._value_to_string_safe(value) + return super().set(section, option, value) + @needs_values @set_dirty_and_flush_changes def set_value(self, section: str, option: str, value: Union[str, bytes, int, float, bool]) -> "GitConfigParser": @@ -902,9 +920,10 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) - self.set(section, option, self._value_to_string(value)) + super().set(section, option, value_str) return self @needs_values @@ -929,9 +948,10 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) - self._sections[section].add(option, self._value_to_string(value)) + self._sections[section].add(option, value_str) return self def rename_section(self, section: str, new_name: str) -> "GitConfigParser": diff --git a/test/test_config.py b/test/test_config.py index 11ea52d16..a9dcdb087 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -150,6 +150,39 @@ def test_config_value_with_trailing_new_line(self): git_config = GitConfigParser(config_file) git_config.read() # This should not throw an exception + @with_rw_directory + def test_set_value_rejects_config_injection(self, rw_dir): + config_path = osp.join(rw_dir, "config") + payload = "foo\n[core]\nhooksPath=/tmp/hooks" + + with GitConfigParser(config_path, read_only=False) as git_config: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", "name", payload) + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertFalse(git_config.has_section("user")) + 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") + bad_values = ("foo\rbar", "foo\nbar", "foo\x00bar", b"foo\nbar") + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_value in bad_values: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set("user", "name", bad_value) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", "name", bad_value) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value("user", "name", bad_value) + + 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") + def test_base(self): path_repo = fixture_path("git_config") path_global = fixture_path("git_config_global")