fix: restore strict python validation (#15)

Author: berisher

README.md

@@ -233,7 +233,7 @@ cargo fuzz run validate_with_python
 ### Available Fuzz Targets
 
 - **`all_protocols`**: Fast fuzzing of all protocols (0-5) with structural validation (~5000-10000 execs/sec)
-- **`validate_with_python`**: Comprehensive validation with Python's `pickletools.genops()` (same logic as `scripts/validate-pickles.py`) including mutation testing (~100-500 execs/sec)
+- **`validate_with_python`**: Comprehensive validation with Python's `pickletools.dis()` plus a whole-file STOP boundary check (same logic as `scripts/validate-pickles.py`) including mutation testing (~100-500 execs/sec)
 
 ### Recommended Workflow
 

fuzz/README.md

@@ -37,8 +37,8 @@ cargo fuzz run all_protocols -- -max_total_time=3600
 - Opcode emission logic
 
 ### 2. `validate_with_python` - Thorough Python Validation
-**Purpose**: Comprehensive validation with Python's `pickletools.genops()`  
-**Validation**: Full structural validation via Python interpreter (uses same logic as `scripts/validate-pickles.py`)  
+**Purpose**: Comprehensive validation with Python's `pickletools.dis()` plus a whole-file STOP boundary check  
+**Validation**: Full structural validation via Python interpreter with no trailing bytes after STOP (uses same logic as `scripts/validate-pickles.py`)  
 **Speed**: ~100-500 execs/sec (subprocess overhead)  
 **Use**: Ensuring generated pickles are structurally valid and parseable by Python
 
@@ -51,7 +51,7 @@ cargo fuzz run validate_with_python -- -max_total_time=1800
 - Opcode range configuration (min/max opcodes)
 - Mutation system with all mutators
 - Mutation rate configuration
-- Python compatibility via `pickletools.genops()` validation
+- Python compatibility via strict whole-file `pickletools` validation
 
 **Note**: This target spawns Python subprocesses to validate each generated pickle using the same validation logic as `scripts/validate-pickles.py`.
 

fuzz/fuzz_targets/validate_with_python.rs

@@ -24,8 +24,9 @@
 //! - arbitrary data seed for generation
 //!
 //! the generated pickles are validated using Python's `pickletools.dis()`
-//!  to ensure they are structurally valid and can be parsed by the reference
-//! implementation. this catches bugs in:
+//! plus a whole-file STOP boundary check to ensure they are structurally
+//! valid, fully consumed, and parseable by the reference implementation.
+//! this catches bugs in:
 //! - opcode emission logic
 //! - stack simulation
 //! - protocol version handling
@@ -52,39 +53,54 @@
 //! Generated pickles must:
 //! 1. be non-empty
 //! 2. end with STOP opcode (0x2e / '.')
-//! 3. successfully validate with `pickletools.dis()` which checks:
+//! 3. successfully validate with Python, which checks:
 //!    - all opcodes parse correctly
 //!    - stack has exactly 1 item before STOP
+//!    - no trailing bytes remain after STOP
 //!    - no invalid operations (via Python subprocess)
 #![no_main]
 
 use libfuzzer_sys::fuzz_target;
-use pickle_fuzzer::{Generator, Version};
 use pickle_fuzzer::mutators::{
-    BitFlipMutator, BoundaryMutator, OffByOneMutator,
-    StringLengthMutator, CharacterMutator,
+    BitFlipMutator, BoundaryMutator, CharacterMutator, OffByOneMutator, StringLengthMutator,
 };
-use std::process::{Command, Stdio};
+use pickle_fuzzer::{Generator, Version};
 use std::io::Write;
+use std::process::{Command, Stdio};
+
+const STRICT_PICKLETOOLS_VALIDATOR: &str = r#"import io
+import pickletools
+import sys
+
+data = sys.stdin.buffer.read()
+stop_pos = None
+for _opcode, _arg, pos in pickletools.genops(data):
+    stop_pos = pos
+if stop_pos is None:
+    raise ValueError("pickle exhausted before seeing STOP")
+if stop_pos + 1 != len(data):
+    raise ValueError(f"trailing bytes after STOP: {len(data) - (stop_pos + 1)}")
+pickletools.dis(data, out=io.StringIO())
+"#;
 
-/// validate pickle using Python's pickletools.dis() which checks stack state after STOP
+/// validate pickle using Python's pickletools plus a whole-file STOP check
 fn validate_with_python(pickle_bytes: &[u8]) -> bool {
     let mut child = match Command::new("python3")
         .arg("-c")
-        .arg("import sys, pickletools, io; pickletools.dis(sys.stdin.buffer.read(), out=io.StringIO())")
+        .arg(STRICT_PICKLETOOLS_VALIDATOR)
         .stdin(Stdio::piped())
         .stdout(Stdio::null())
         .stderr(Stdio::piped())
         .spawn()
     {
         Ok(child) => child,
-        Err(_) => return true, // Skip validation if Python unavailable
+        Err(err) => panic!("validate_with_python fuzz target requires python3 on PATH: {err}"),
     };
-    
+
     if let Some(mut stdin) = child.stdin.take() {
         let _ = stdin.write_all(pickle_bytes);
     }
-    
+
     let output = child.wait_with_output().unwrap();
     output.status.success()
 }
@@ -98,15 +114,15 @@ fuzz_target!(|data: &[u8]| {
     // byte 0: protocol version (0-5)
     let protocol = (data[0] % 6) as usize;
     let version = Version::try_from(protocol).unwrap();
-    
+
     let gen = Generator::new(version);
 
     // bytes 1-4: opcode range (min/max)
     let min_opcodes = u16::from_le_bytes([data[1], data[2]]) as usize;
     let max_opcodes = u16::from_le_bytes([data[3], data[4]]) as usize;
-    
+
     // ensure valid range and cap at 1000 opcodes to prevent stack overflow
-    let mut gen = if min_opcodes > max_opcodes  {
+    let mut gen = if min_opcodes > max_opcodes {
         if min_opcodes > 1000 {
             return;
         }
@@ -148,8 +164,12 @@ fuzz_target!(|data: &[u8]| {
     if let Ok(pickle) = gen.generate_from_arbitrary(&data[7..]) {
         // basic structural validation
         assert!(!pickle.is_empty(), "generated pickle must not be empty");
-        assert_eq!(pickle[pickle.len() - 1], b'.', "pickle must end with STOP opcode");
-        
+        assert_eq!(
+            pickle[pickle.len() - 1],
+            b'.',
+            "pickle must end with STOP opcode"
+        );
+
         // validate with Python's pickletools
         assert!(
             validate_with_python(&pickle),

scripts/validate-pickles.py

@@ -31,17 +31,28 @@
 DEFAULT_EXTENSIONS: tuple[str, ...] = (".pkl", ".pickle")
 
 
+def is_confined_to_root(path: Path, root: Path) -> bool:
+    try:
+        path.resolve().relative_to(root)
+    except (OSError, RuntimeError, ValueError):
+        return False
+    return True
+
+
 def iter_pickles(paths: Sequence[str], extensions: Sequence[str]) -> Iterable[Path]:
     exts = tuple(ext.lower() for ext in extensions)
     for raw in paths:
         path = Path(raw)
         if not path.exists():
             raise FileNotFoundError(f"Path not found: {path}")
         if path.is_dir():
+            root = path.resolve()
             yield from (
                 child
                 for child in path.rglob("*")
-                if child.is_file() and child.suffix.lower() in exts
+                if child.is_file()
+                and child.suffix.lower() in exts
+                and is_confined_to_root(child, root)
             )
         elif path.is_file():
             if path.suffix.lower() in exts:
@@ -52,14 +63,21 @@ def iter_pickles(paths: Sequence[str], extensions: Sequence[str]) -> Iterable[Pa
 
 def validate_pickle(data: bytes) -> tuple[bool, str | None]:
     try:
-        for _opcode, _arg, _pos in pickletools.genops(data):
-            pass
+        disassemble(data)
     except Exception as exc:  # pragma: no cover - defensive
         return False, str(exc)
     return True, None
 
 
 def disassemble(data: bytes) -> str:
+    stop_pos = None
+    for _opcode, _arg, pos in pickletools.genops(data):
+        stop_pos = pos
+    if stop_pos is None:
+        raise ValueError("pickle exhausted before seeing STOP")
+    if stop_pos + 1 != len(data):
+        raise ValueError(f"trailing bytes after STOP: {len(data) - (stop_pos + 1)}")
+
     buffer = io.StringIO()
     pickletools.dis(data, out=buffer)
     return buffer.getvalue()
@@ -79,7 +97,7 @@ def parse_args(argv: Sequence[str]) -> argparse.Namespace:
         "--extension",
         dest="extensions",
         action="append",
-        default=list(DEFAULT_EXTENSIONS),
+        default=None,
         help="File extension to include (default: .pkl, .pickle). Specify multiple times.",
     )
     parser.add_argument(
@@ -98,7 +116,8 @@ def parse_args(argv: Sequence[str]) -> argparse.Namespace:
 
 def main(argv: Sequence[str] | None = None) -> int:
     ns = parse_args(sys.argv[1:] if argv is None else argv)
-    paths = sorted(set(iter_pickles(ns.paths, ns.extensions)))
+    extensions = ns.extensions if ns.extensions is not None else DEFAULT_EXTENSIONS
+    paths = sorted(set(iter_pickles(ns.paths, extensions)))
 
     if not paths:
         print("No pickle files matched the provided paths and extensions.", file=sys.stderr)