Merge pull request #128 from devdanzin/oom-dedup-rename-failure

oom_dedup: never let a dedupe failure abort the session keep/rename

Author: devdanzin

fusil/python/__init__.py

@@ -603,9 +603,14 @@ def _oom_keep_policy(self, session):
         try:
             with open(os.path.join(session_dir, "stdout"), errors="replace") as fh:
                 text = fh.read()
-        except OSError:
+            return self._deduper.decide(text, source_path=os.path.join(session_dir, "source.py"))
+        except Exception as err:
+            # Never let a dedupe failure abort the session's keep/rename (deinit): keep the
+            # crash dir unlabelled rather than lose it. (decide() resolves segvs via gdb, whose
+            # captured output can be binary -- a past UnicodeDecodeError here left crash dirs
+            # stuck as session-NNNN instead of renamed.)
+            self.error("OOM keep-policy failed (%s); keeping crash dir unlabelled" % err)
             return True, None
-        return self._deduper.decide(text, source_path=os.path.join(session_dir, "source.py"))
 
     def exit(self, keep_log: bool = True) -> None:
         """Clean up and exit the fuzzer, printing runtime statistics."""

fusil/python/oom_dedup.py

@@ -277,12 +277,17 @@ def gdb_crash_site(python_bin, source_path, timeout=120, drop_uid=None, drop_gid
             ],
             capture_output=True,
             text=True,
+            # The re-run prints the fuzzed program's own stdout into gdb's captured output,
+            # which is frequently binary (e.g. a gzip/zlib vehicle emits 0x1f 0x8b ...). Decode
+            # tolerantly so it never raises UnicodeDecodeError -- otherwise the exception would
+            # escape decide() and abort the session's keep/rename in deinit.
+            errors="replace",
             timeout=timeout,
             cwd=cwd,
             env={**os.environ, "ASAN_OPTIONS": "detect_leaks=0:abort_on_error=0"},
             **drop,
         ).stdout
-    except (OSError, subprocess.SubprocessError):
+    except (OSError, ValueError, subprocess.SubprocessError):
         return []
     return extract_sites_from_bt(out)
 
@@ -336,22 +341,29 @@ def __init__(
         self.kept = collections.Counter()  # bug -> directories kept
 
     def _resolve(self, source_path):
-        """Return a list of 'func@file:line' chain frames (normalising the resolver)."""
-        r = (
-            self._resolver(source_path)
-            if self._resolver is not None
-            else (
-                gdb_crash_site(
-                    self.python_bin,
-                    source_path,
-                    self.gdb_timeout,
-                    drop_uid=self.drop_uid,
-                    drop_gid=self.drop_gid,
+        """Return a list of 'func@file:line' chain frames (normalising the resolver).
+
+        Any failure in the resolver is swallowed (-> no chain): segv resolution is a
+        best-effort enrichment, and decide() must never raise into the caller's keep/rename.
+        """
+        try:
+            r = (
+                self._resolver(source_path)
+                if self._resolver is not None
+                else (
+                    gdb_crash_site(
+                        self.python_bin,
+                        source_path,
+                        self.gdb_timeout,
+                        drop_uid=self.drop_uid,
+                        drop_gid=self.drop_gid,
+                    )
+                    if (self.python_bin and source_path)
+                    else None
                 )
-                if (self.python_bin and source_path)
-                else None
             )
-        )
+        except Exception:
+            return []
         if not r:
             return []
         return [r] if isinstance(r, str) else list(r)

tests/python/test_oom_dedup.py

@@ -328,6 +328,16 @@ def test_unresolvable_segv_kept_as_segv(self):
         d = self._deduper(lambda sp: None)
         self.assertEqual(d.decide(SEGV, source_path="s"), (True, "oomSEGV"))
 
+    def test_resolver_exception_does_not_break_decide(self):
+        # A resolver that raises (e.g. gdb's captured output is binary -> UnicodeDecodeError)
+        # must not propagate out of decide(): segv resolution is best-effort, and a raise here
+        # previously aborted the session's keep/rename in deinit, leaving dirs as session-NNNN.
+        def boom(sp):
+            raise UnicodeDecodeError("utf-8", b"\x8b", 0, 1, "invalid start byte")
+
+        d = self._deduper(boom)
+        self.assertEqual(d.decide(SEGV, source_path="s"), (True, "oomSEGV"))
+
     def test_resolved_known_segv_prunes_over_cap(self):
         d = self._deduper(
             lambda sp: "dictiter_dealloc@Objects/dictobject.c:5532", keep=1, prune=True

tests/python/test_oom_dedup_wiring.py

@@ -36,7 +36,12 @@ def _fake_self(self, prune=False, keep=5):
         with os.fdopen(fd, "w") as fh:
             fh.write(SNAPSHOT)
         self.addCleanup(os.unlink, path)
-        return SimpleNamespace(_deduper=Deduper(path, keep=keep, prune=prune))
+        # `error` mirrors the Application logger the real Fuzzer has; the keep-policy uses it
+        # to report a dedupe failure before falling back to (True, None).
+        return SimpleNamespace(
+            _deduper=Deduper(path, keep=keep, prune=prune),
+            error=lambda *a, **k: None,
+        )
 
     def _session(self, text):
         d = tempfile.mkdtemp()