From 7b684cd43cf0f9c54adb8a8def54fa07f5cfd145 Mon Sep 17 00:00:00 2001
From: David Lakin <github@themoderndev.com>
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 <github@themoderndev.com>
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 <github@themoderndev.com>
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 <github@themoderndev.com>
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 <github@themoderndev.com>
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)