Fuzzing: Fix broken test for Git submodule handling

DaveLak · DaveLak · commit 7751d0b98920 · 2025-01-10T14:12:53.000-05:00
Ensured submodule names, paths, and commit messages are sanitized to
avoid invalid states that are expected to cause exceptions and should
not halt the fuzzer.

In particular, the changes here:
- Sanitized inputs for submodule names, paths, and commit messages.
- Added validation for submodule SHA and path integrity.
diff --git a/fuzzing/fuzz-targets/fuzz_submodule.py b/fuzzing/fuzz-targets/fuzz_submodule.py
@@ -9,11 +9,17 @@
     get_max_filename_length,
 )
 
-# Setup the git environment
+# Setup the Git environment
 setup_git_environment()
 from git import Repo, GitCommandError, InvalidGitRepositoryError
 
 
+def sanitize_input(input_str, max_length=255):
+    """Sanitize and truncate inputs to avoid invalid Git operations."""
+    sanitized = "".join(ch for ch in input_str if ch.isalnum() or ch in ("-", "_", "."))
+    return sanitized[:max_length]
+
+
 def TestOneInput(data):
     fdp = atheris.FuzzedDataProvider(data)
 
@@ -24,12 +30,23 @@ def TestOneInput(data):
         try:
             with tempfile.TemporaryDirectory() as submodule_temp_dir:
                 sub_repo = Repo.init(submodule_temp_dir, bare=fdp.ConsumeBool())
-                sub_repo.index.commit(fdp.ConsumeUnicodeNoSurrogates(fdp.ConsumeIntInRange(1, 512)))
+                commit_message = sanitize_input(fdp.ConsumeUnicodeNoSurrogates(fdp.ConsumeIntInRange(1, 512)))
+                sub_repo.index.commit(commit_message)
 
-                submodule_name = fdp.ConsumeUnicodeNoSurrogates(
-                    fdp.ConsumeIntInRange(1, max(1, get_max_filename_length(repo.working_tree_dir)))
+                submodule_name = sanitize_input(
+                    fdp.ConsumeUnicodeNoSurrogates(
+                        fdp.ConsumeIntInRange(1, get_max_filename_length(repo.working_tree_dir))
+                    )
                 )
-                submodule_path = os.path.join(repo.working_tree_dir, submodule_name)
+
+                submodule_path = os.path.relpath(
+                    os.path.join(repo.working_tree_dir, submodule_name),
+                    start=repo.working_tree_dir,
+                )
+
+                # Ensure submodule_path is valid
+                if not submodule_name or submodule_name.startswith("/") or ".." in submodule_name:
+                    return -1  # Reject invalid input so they are not added to the corpus
 
                 submodule = repo.create_submodule(submodule_name, submodule_path, url=sub_repo.git_dir)
                 repo.index.commit("Added submodule")
@@ -39,25 +56,38 @@ def TestOneInput(data):
                     value_length = fdp.ConsumeIntInRange(1, max(1, fdp.remaining_bytes()))
 
                     writer.set_value(
-                        fdp.ConsumeUnicodeNoSurrogates(key_length), fdp.ConsumeUnicodeNoSurrogates(value_length)
+                        sanitize_input(fdp.ConsumeUnicodeNoSurrogates(key_length)),
+                        sanitize_input(fdp.ConsumeUnicodeNoSurrogates(value_length)),
                     )
                     writer.release()
 
-                submodule.update(init=fdp.ConsumeBool(), dry_run=fdp.ConsumeBool(), force=fdp.ConsumeBool())
+                submodule.update(
+                    init=fdp.ConsumeBool(),
+                    dry_run=fdp.ConsumeBool(),
+                    force=fdp.ConsumeBool(),
+                )
+
                 submodule_repo = submodule.module()
 
-                new_file_name = fdp.ConsumeUnicodeNoSurrogates(
-                    fdp.ConsumeIntInRange(1, max(1, get_max_filename_length(submodule_repo.working_tree_dir)))
+                new_file_name = sanitize_input(
+                    fdp.ConsumeUnicodeNoSurrogates(
+                        fdp.ConsumeIntInRange(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])
                 submodule_repo.index.commit("Added new file to submodule")
 
                 repo.submodule_update(recursive=fdp.ConsumeBool())
-                submodule_repo.head.reset(commit="HEAD~1", working_tree=fdp.ConsumeBool(), head=fdp.ConsumeBool())
-                # Use fdp.PickValueInList to ensure at least one of 'module' or 'configuration' is True
+                submodule_repo.head.reset(
+                    commit="HEAD~1",
+                    working_tree=fdp.ConsumeBool(),
+                    head=fdp.ConsumeBool(),
+                )
+
                 module_option_value, configuration_option_value = fdp.PickValueInList(
                     [(True, False), (False, True), (True, True)]
                 )
@@ -82,12 +112,7 @@ def TestOneInput(data):
         ):
             return -1
         except Exception as e:
-            if isinstance(e, ValueError) and "embedded null byte" in str(e):
-                return -1
-            elif isinstance(e, OSError) and "File name too long" in str(e):
-                return -1
-            else:
-                return handle_exception(e)
+            return handle_exception(e)
 
 
 def main():
