Content-Length: 10153 | pFad | http://github.com/gitpython-developers/GitPython/pull/1922.patch

thub.com From 7b684cd43cf0f9c54adb8a8def54fa07f5cfd145 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Thu, 30 May 2024 10:34:49 -0400 Subject: [PATCH 1/5] Add graceful handling of expected exceptions in `fuzz_submodule.py` Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69350 **`IsADirectoryError`** Fuzzer provided input data can sometimes produce filenames that look like directories and raise `IsADirectoryError` exceptions which crash the fuzzer. This commit catches those cases and returns -1 to instruct libfuzzer that the inputs are not valuable to add to the corpus. **`FileExistsError`** Similar to the above, this is a possible exception case produced by the fuzzed data and not a bug so its handled the same. --- fuzzing/fuzz-targets/fuzz_submodule.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py index ddcbaa00f..9406fc68b 100644 --- a/fuzzing/fuzz-targets/fuzz_submodule.py +++ b/fuzzing/fuzz-targets/fuzz_submodule.py @@ -67,7 +67,15 @@ def TestOneInput(data): ) repo.index.commit(f"Removed submodule {submodule_name}") - except (ParsingError, GitCommandError, InvalidGitRepositoryError, FileNotFoundError, BrokenPipeError): + except ( + ParsingError, + GitCommandError, + InvalidGitRepositoryError, + FileNotFoundError, + FileExistsError, + IsADirectoryError, + BrokenPipeError, + ): return -1 except (ValueError, OSError) as e: expected_messages = [ From 6c00ce602eb19eda342e827a25d005610ce92fa8 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Thu, 30 May 2024 13:53:42 -0400 Subject: [PATCH 2/5] Improve file name generation to prevent "File name too long" `OSError`'s Adds a utility function to limit the maximum file name legnth produced by the fuzzer to a max size dictated by the host its run on. --- fuzzing/fuzz-targets/fuzz_submodule.py | 13 ++++++------- fuzzing/fuzz-targets/utils.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py index 9406fc68b..817ce8f98 100644 --- a/fuzzing/fuzz-targets/fuzz_submodule.py +++ b/fuzzing/fuzz-targets/fuzz_submodule.py @@ -3,7 +3,7 @@ import os import tempfile from configparser import ParsingError -from utils import is_expected_exception_message +from utils import is_expected_exception_message, get_max_filename_length if getattr(sys, "frozen", False) and hasattr(sys, "_MEIPASS"): path_to_bundled_git_binary = os.path.abspath(os.path.join(os.path.dirname(__file__), "git")) @@ -42,12 +42,12 @@ def TestOneInput(data): writer.release() submodule.update(init=fdp.ConsumeBool(), dry_run=fdp.ConsumeBool(), force=fdp.ConsumeBool()) - submodule_repo = submodule.module() - new_file_path = os.path.join( - submodule_repo.working_tree_dir, - f"new_file_{fdp.ConsumeUnicodeNoSurrogates(fdp.ConsumeIntInRange(1, 512))}", + + new_file_name = fdp.ConsumeUnicodeNoSurrogates( + fdp.ConsumeIntInRange(1, max(1, get_max_filename_length(submodule_repo.working_tree_dir))) ) + new_file_path = os.path.join(submodule_repo.working_tree_dir, new_file_name) with open(new_file_path, "wb") as new_file: new_file.write(fdp.ConsumeBytes(fdp.ConsumeIntInRange(1, 512))) submodule_repo.index.add([new_file_path]) @@ -77,14 +77,13 @@ def TestOneInput(data): BrokenPipeError, ): return -1 - except (ValueError, OSError) as e: + except ValueError as e: expected_messages = [ "SHA is empty", "Reference at", "embedded null byte", "This submodule instance does not exist anymore", "cmd stdin was empty", - "File name too long", ] if is_expected_exception_message(e, expected_messages): return -1 diff --git a/fuzzing/fuzz-targets/utils.py b/fuzzing/fuzz-targets/utils.py index 42faa8eb0..86f049341 100644 --- a/fuzzing/fuzz-targets/utils.py +++ b/fuzzing/fuzz-targets/utils.py @@ -1,4 +1,5 @@ import atheris # pragma: no cover +import os from typing import List # pragma: no cover @@ -20,3 +21,17 @@ def is_expected_exception_message(exception: Exception, error_message_list: List if error.lower() in exception_message: return True return False + + +@atheris.instrument_func +def get_max_filename_length(path: str) -> int: + """ + Get the maximum filename length for the filesystem containing the given path. + + Args: + path (str): The path to check the filesystem for. + + Returns: + int: The maximum filename length. + """ + return os.pathconf(path, "PC_NAME_MAX") From 2a2294f9d1e46d9bbe11cd2031d62e5441fe19c4 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Thu, 30 May 2024 14:02:27 -0400 Subject: [PATCH 3/5] Improve `fuzz_submodule.py` coverage & efficacy The fuzzer was having trouble analyzing `fuzz_submodule.py` when using the `atheris.instrument_imports()` context manager. Switching to `atheris.instrument_all()` instead slightly increases the startup time for the fuzzer, but significantly improves the fuzzing engines ability to identify new coverage. The changes here also disable warnings that are logged to `stdout` from the SUT. These warnings are expected to happen with some inputs and clutter the fuzzer output logs. They can be optionally re-enabled for debugging by passing a flag o the Python interpreter command line or setting the `PYTHONWARNINGS` environment variable. --- fuzzing/fuzz-targets/fuzz_submodule.py | 16 +++++++++++++--- fuzzing/fuzz-targets/utils.py | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py index 817ce8f98..53f5a7884 100644 --- a/fuzzing/fuzz-targets/fuzz_submodule.py +++ b/fuzzing/fuzz-targets/fuzz_submodule.py @@ -4,13 +4,22 @@ import tempfile from configparser import ParsingError from utils import is_expected_exception_message, get_max_filename_length +from git import Repo, GitCommandError, InvalidGitRepositoryError -if getattr(sys, "frozen", False) and hasattr(sys, "_MEIPASS"): +if getattr(sys, "frozen", False) and hasattr(sys, "_MEIPASS"): # pragma: no cover path_to_bundled_git_binary = os.path.abspath(os.path.join(os.path.dirname(__file__), "git")) os.environ["GIT_PYTHON_GIT_EXECUTABLE"] = path_to_bundled_git_binary -with atheris.instrument_imports(): - from git import Repo, GitCommandError, InvalidGitRepositoryError +if not sys.warnoptions: # pragma: no cover + # The warnings filter below can be overridden by passing the -W option + # to the Python interpreter command line or setting the `PYTHONWARNINGS` environment variable. + import warnings + import logging + + # Fuzzing data causes some plugins to generate a large number of warnings + # which are not usually interesting and make the test output hard to read, so we ignore them. + warnings.simplefilter("ignore") + logging.getLogger().setLevel(logging.ERROR) def TestOneInput(data): @@ -92,6 +101,7 @@ def TestOneInput(data): def main(): + atheris.instrument_all() atheris.Setup(sys.argv, TestOneInput) atheris.Fuzz() diff --git a/fuzzing/fuzz-targets/utils.py b/fuzzing/fuzz-targets/utils.py index 86f049341..f522d2959 100644 --- a/fuzzing/fuzz-targets/utils.py +++ b/fuzzing/fuzz-targets/utils.py @@ -1,5 +1,5 @@ import atheris # pragma: no cover -import os +import os # pragma: no cover from typing import List # pragma: no cover @@ -24,7 +24,7 @@ def is_expected_exception_message(exception: Exception, error_message_list: List @atheris.instrument_func -def get_max_filename_length(path: str) -> int: +def get_max_filename_length(path: str) -> int: # pragma: no cover """ Get the maximum filename length for the filesystem containing the given path. From 57a56a8a2874d2ab76f4034b9d3c98e09ed7fa35 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Thu, 30 May 2024 14:12:02 -0400 Subject: [PATCH 4/5] Add graceful handling for `NotADirectoryError`s --- fuzzing/fuzz-targets/fuzz_submodule.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py index 53f5a7884..cfd1a6d3f 100644 --- a/fuzzing/fuzz-targets/fuzz_submodule.py +++ b/fuzzing/fuzz-targets/fuzz_submodule.py @@ -83,6 +83,7 @@ def TestOneInput(data): FileNotFoundError, FileExistsError, IsADirectoryError, + NotADirectoryError, BrokenPipeError, ): return -1 From 2b64dee466ed72523684f90a037d604355121df0 Mon Sep 17 00:00:00 2001 From: David Lakin Date: Thu, 30 May 2024 14:17:20 -0400 Subject: [PATCH 5/5] Improve comment wording --- fuzzing/fuzz-targets/fuzz_submodule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py index cfd1a6d3f..92b569949 100644 --- a/fuzzing/fuzz-targets/fuzz_submodule.py +++ b/fuzzing/fuzz-targets/fuzz_submodule.py @@ -16,7 +16,7 @@ import warnings import logging - # Fuzzing data causes some plugins to generate a large number of warnings + # Fuzzing data causes some modules to generate a large number of warnings # which are not usually interesting and make the test output hard to read, so we ignore them. warnings.simplefilter("ignore") logging.getLogger().setLevel(logging.ERROR)

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!