From 47e5738074e7f9acfb64d164206770bbd41685a0 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Mon, 22 Apr 2024 18:48:26 -0400 Subject: [PATCH 1/3] Fix IndexError in GitConfigParser when config value ends in new line Improve the guarding `if` check in `GitConfigParser`'s `string_decode` function to safely handle empty strings and prevent `IndexError`s when accessing string elements. This resolves an IndexError in the `GitConfigParser`'s `.read()` method when the config file contains a quoted value containing a trailing new line. Fixes: https://github.com/gitpython-developers/GitPython/issues/1887 --- fuzzing/fuzz-targets/fuzz_config.py | 8 ++------ git/config.py | 2 +- test/test_config.py | 8 ++++++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/fuzzing/fuzz-targets/fuzz_config.py b/fuzzing/fuzz-targets/fuzz_config.py index 0a06956c8..81dcf9a88 100644 --- a/fuzzing/fuzz-targets/fuzz_config.py +++ b/fuzzing/fuzz-targets/fuzz_config.py @@ -34,12 +34,8 @@ def TestOneInput(data): git_config.read() except (MissingSectionHeaderError, ParsingError, UnicodeDecodeError): return -1 # Reject inputs raising expected exceptions - except (IndexError, ValueError) as e: - if isinstance(e, IndexError) and "string index out of range" in str(e): - # Known possibility that might be patched - # See: https://github.com/gitpython-developers/GitPython/issues/1887 - pass - elif isinstance(e, ValueError) and "embedded null byte" in str(e): + except ValueError as e: + if isinstance(e, ValueError) and "embedded null byte" in str(e): # The `os.path.expanduser` function, which does not accept strings # containing null bytes might raise this. return -1 diff --git a/git/config.py b/git/config.py index 3ce9b123f..c9b49684c 100644 --- a/git/config.py +++ b/git/config.py @@ -452,7 +452,7 @@ def _read(self, fp: Union[BufferedReader, IO[bytes]], fpname: str) -> None: e = None # None, or an exception. def string_decode(v: str) -> str: - if v[-1] == "\\": + if v and v[-1] == "\\": v = v[:-1] # END cut trailing escapes to prevent decode error diff --git a/test/test_config.py b/test/test_config.py index 0911d0262..92997422d 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -142,6 +142,14 @@ def test_multi_line_config(self): ) self.assertEqual(len(config.sections()), 23) + def test_config_value_with_trailing_new_line(self): + config_content = b'[section-header]\nkey:"value\n"' + config_file = io.BytesIO(config_content) + config_file.name = "multiline_value.config" + + git_config = GitConfigParser(config_file) + git_config.read() # This should not throw an exception + def test_base(self): path_repo = fixture_path("git_config") path_global = fixture_path("git_config_global") From c2283f6e3566606300f64c44a12197f0b65f0d71 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Apr 2024 08:21:58 +0200 Subject: [PATCH 2/3] Avoid unnecessary isinstance check --- fuzzing/fuzz-targets/fuzz_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuzzing/fuzz-targets/fuzz_config.py b/fuzzing/fuzz-targets/fuzz_config.py index 81dcf9a88..80ab7b08d 100644 --- a/fuzzing/fuzz-targets/fuzz_config.py +++ b/fuzzing/fuzz-targets/fuzz_config.py @@ -35,7 +35,7 @@ def TestOneInput(data): except (MissingSectionHeaderError, ParsingError, UnicodeDecodeError): return -1 # Reject inputs raising expected exceptions except ValueError as e: - if isinstance(e, ValueError) and "embedded null byte" in str(e): + if "embedded null byte" in str(e): # The `os.path.expanduser` function, which does not accept strings # containing null bytes might raise this. return -1 From 1a0ab5bbdaa9df5d04d1b6946af419492b650fce Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 26 Apr 2024 07:15:13 +0200 Subject: [PATCH 3/3] Use endswith() for more clarity --- git/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index c9b49684c..de3508360 100644 --- a/git/config.py +++ b/git/config.py @@ -452,7 +452,7 @@ def _read(self, fp: Union[BufferedReader, IO[bytes]], fpname: str) -> None: e = None # None, or an exception. def string_decode(v: str) -> str: - if v and v[-1] == "\\": + if v and v.endswith("\\"): v = v[:-1] # END cut trailing escapes to prevent decode error