Skip to content

Commit a31df76

Browse files
fix: avoid pickle meta-path source probing (#1493)
* fix: avoid pickle meta-path source probing * fix: avoid executable pickle source finders * test: use explicit call graph helper import * fix: honor safe import resolution precedence * fix: harden pickle source resolution precedence
1 parent 463bc2c commit a31df76

4 files changed

Lines changed: 565 additions & 41 deletions

File tree

packages/modelaudit-picklescan/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5858
- bound native pickle state simulation for tracked dictionaries, memo clones,
5959
dotted globals, and recursive mappings
6060
- fail closed when encoded nested-pickle probe candidates exhaust the analysis cap
61+
- avoid custom meta-path finder calls during pickle call-graph source probing
6162
- prevent call-graph alias cycles from hanging scans
6263
- detect nested brace-format lookups that reach tracked `defaultdict` factories
6364
- avoid `str.format` false positives when a `ChainMap` shadows a `defaultdict`

packages/modelaudit-picklescan/src/modelaudit_picklescan/call_graph.py

Lines changed: 143 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import ast
6+
import fnmatch
67
import hashlib
78
import os
89
import sys
@@ -14,9 +15,22 @@
1415
from contextvars import ContextVar
1516
from dataclasses import dataclass, field
1617
from functools import lru_cache
17-
from importlib.machinery import EXTENSION_SUFFIXES, BuiltinImporter, FrozenImporter, ModuleSpec, PathFinder
18+
from importlib.machinery import (
19+
BYTECODE_SUFFIXES,
20+
EXTENSION_SUFFIXES,
21+
SOURCE_SUFFIXES,
22+
BuiltinImporter,
23+
ExtensionFileLoader,
24+
FileFinder,
25+
FrozenImporter,
26+
ModuleSpec,
27+
PathFinder,
28+
SourceFileLoader,
29+
SourcelessFileLoader,
30+
)
1831
from pathlib import Path
1932
from typing import Any, Protocol, TypeVar, cast
33+
from zipimport import zipimporter
2034

2135
# Bound per-pass import/callable fan-out for untrusted inputs. The 32-reference
2236
# cap has kept call-graph enrichment useful while preventing pathological scan
@@ -33,6 +47,7 @@
3347
_MAX_ASSIGNMENT_ALIASES = 128
3448
_MAX_ASSIGNMENT_ALIAS_PASSES = 256
3549
_MAX_FUNCTION_INSTANCE_ALIASES = 32
50+
_TRUSTED_PATH_HOOKS = tuple(sys.path_hooks)
3651
_MAX_CLASS_INSTANCE_ALIASES = 128
3752
_MAX_INHERITED_CLASS_METHODS = 128
3853
_MAX_WILDCARD_IMPORTS = 16
@@ -1007,20 +1022,13 @@ def _call_graph_source_unavailable_reason(module_name: str) -> str | None:
10071022
return "source_parse_error"
10081023
return None
10091024

1010-
if module_name.split(".", maxsplit=1)[0] in sys.builtin_module_names:
1011-
return None
1012-
10131025
try:
10141026
spec = _find_module_spec_without_imports(module_name)
10151027
except Exception:
10161028
return "source_unavailable"
10171029
if spec is None:
1018-
try:
1019-
spec = _find_meta_path_module_spec_without_imports(module_name)
1020-
except Exception:
1021-
return "source_unavailable"
1022-
if spec is None:
1023-
return None
1030+
# Module names come from pickle metadata; do not consult executable custom meta-path finders.
1031+
return "source_unavailable"
10241032
if spec.origin in {"built-in", "frozen"}:
10251033
return None
10261034
if spec.origin is not None and any(spec.origin.endswith(suffix) for suffix in EXTENSION_SUFFIXES):
@@ -1033,11 +1041,37 @@ def _find_module_spec_without_imports(module_name: str) -> ModuleSpec | None:
10331041
if not parts or any(not part or "/" in part or "\\" in part for part in parts):
10341042
return None
10351043

1036-
search_path: list[str] | None = None
1044+
loaded_module = sys.modules.get(module_name)
1045+
loaded_spec = getattr(loaded_module, "__spec__", None)
1046+
if isinstance(loaded_spec, ModuleSpec):
1047+
return loaded_spec
1048+
1049+
if not _untrusted_meta_path_finder_precedes(BuiltinImporter, module_name):
1050+
builtin_spec = BuiltinImporter.find_spec(module_name)
1051+
if builtin_spec is not None:
1052+
return builtin_spec
1053+
1054+
if not _untrusted_meta_path_finder_precedes(FrozenImporter, module_name):
1055+
frozen_spec = FrozenImporter.find_spec(module_name)
1056+
if frozen_spec is not None:
1057+
return frozen_spec
1058+
1059+
if _untrusted_meta_path_finder_precedes(PathFinder, module_name) or _has_untrusted_path_hook():
1060+
return None
1061+
1062+
return _find_standard_filesystem_spec(module_name)
1063+
1064+
1065+
def _find_standard_filesystem_spec(module_name: str) -> ModuleSpec | None:
1066+
parts = module_name.split(".")
1067+
if not parts or any(not part or "/" in part or "\\" in part for part in parts):
1068+
return None
1069+
1070+
search_path = [str(Path(entry or os.getcwd())) for entry in sys.path]
10371071
spec: ModuleSpec | None = None
10381072
for index in range(len(parts)):
10391073
qualified_name = ".".join(parts[: index + 1])
1040-
spec = PathFinder.find_spec(qualified_name, search_path)
1074+
spec = _find_standard_path_spec(qualified_name, search_path)
10411075
if spec is None:
10421076
return None
10431077
if index == len(parts) - 1:
@@ -1049,18 +1083,91 @@ def _find_module_spec_without_imports(module_name: str) -> ModuleSpec | None:
10491083
return spec
10501084

10511085

1052-
def _find_meta_path_module_spec_without_imports(module_name: str) -> ModuleSpec | None:
1053-
"""Consult non-standard meta path finders without importing parent packages."""
1086+
def _matches_loaded_finder_type(finder: object, module_name: str, type_name: str) -> bool:
1087+
module = sys.modules.get(module_name)
1088+
finder_type = getattr(module, type_name, None) if module is not None else None
1089+
return isinstance(finder_type, type) and type(finder) is finder_type
1090+
1091+
1092+
def _known_meta_path_finder_cannot_handle(finder: object, module_name: str) -> bool:
1093+
root_name = module_name.split(".", maxsplit=1)[0]
1094+
if _matches_loaded_finder_type(finder, "_distutils_hack", "DistutilsMetaFinder"):
1095+
return root_name not in {"distutils", "pip", "test"}
1096+
1097+
if _matches_loaded_finder_type(finder, "_virtualenv", "_Finder"):
1098+
virtualenv_module = sys.modules.get("_virtualenv")
1099+
patched_modules = getattr(virtualenv_module, "_DISTUTILS_PATCH", ()) if virtualenv_module is not None else ()
1100+
return module_name not in patched_modules
1101+
1102+
if _matches_loaded_finder_type(finder, "_pytest.assertion.rewrite", "AssertionRewritingHook"):
1103+
if module_name == "conftest":
1104+
return False
1105+
must_rewrite = getattr(finder, "_must_rewrite", ())
1106+
if any(module_name == name or module_name.startswith(f"{name}.") for name in must_rewrite):
1107+
return False
1108+
patterns = getattr(finder, "fnpats", ())
1109+
module_filename = f"{module_name.rsplit('.', maxsplit=1)[-1]}.py"
1110+
return all(not fnmatch.fnmatchcase(module_filename, pattern) for pattern in patterns)
1111+
1112+
return False
1113+
1114+
1115+
def _untrusted_meta_path_finder_precedes(target: object, module_name: str) -> bool:
10541116
for finder in sys.meta_path:
1117+
if finder is target:
1118+
return False
10551119
if finder is BuiltinImporter or finder is FrozenImporter or finder is PathFinder:
10561120
continue
1057-
find_spec = getattr(finder, "find_spec", None)
1058-
if find_spec is None:
1121+
if _known_meta_path_finder_cannot_handle(finder, module_name):
1122+
continue
1123+
return True
1124+
return True
1125+
1126+
1127+
def _is_standard_path_hook(hook: object) -> bool:
1128+
return hook is zipimporter or any(hook is trusted_hook for trusted_hook in _TRUSTED_PATH_HOOKS)
1129+
1130+
1131+
def _has_untrusted_path_hook() -> bool:
1132+
if any(not _is_standard_path_hook(hook) for hook in sys.path_hooks):
1133+
return True
1134+
for entry in sys.path:
1135+
cache_key = entry or os.getcwd()
1136+
finder = sys.path_importer_cache.get(cache_key)
1137+
if finder is not None and not isinstance(finder, (FileFinder, zipimporter)):
1138+
return True
1139+
return False
1140+
1141+
1142+
def _find_standard_path_spec(module_name: str, search_path: list[str]) -> ModuleSpec | None:
1143+
namespace_locations: list[str] = []
1144+
loader_details = (
1145+
(ExtensionFileLoader, EXTENSION_SUFFIXES),
1146+
(SourceFileLoader, SOURCE_SUFFIXES),
1147+
(SourcelessFileLoader, BYTECODE_SUFFIXES),
1148+
)
1149+
for entry in search_path:
1150+
try:
1151+
zip_spec = zipimporter(entry).find_spec(module_name)
1152+
except ImportError:
1153+
zip_spec = None
1154+
if zip_spec is not None:
1155+
return zip_spec
1156+
1157+
finder = FileFinder(entry, *loader_details)
1158+
spec = finder.find_spec(module_name)
1159+
if spec is None:
10591160
continue
1060-
spec = find_spec(module_name, None)
1061-
if isinstance(spec, ModuleSpec):
1161+
if spec.loader is not None:
10621162
return spec
1063-
return None
1163+
if spec.submodule_search_locations is not None:
1164+
namespace_locations.extend(spec.submodule_search_locations)
1165+
1166+
if not namespace_locations:
1167+
return None
1168+
namespace_spec = ModuleSpec(module_name, loader=None, is_package=True)
1169+
namespace_spec.submodule_search_locations = namespace_locations
1170+
return namespace_spec
10641171

10651172

10661173
@_register_source_sensitive_cache
@@ -3485,22 +3592,23 @@ def _resolve_module_source(module_name: str) -> Path | None:
34853592
if not parts or any(not part or "/" in part or "\\" in part for part in parts):
34863593
return None
34873594
_track_shared_source_candidates(tuple(parts))
3488-
for entry in sys.path:
3489-
root = Path(entry or os.getcwd())
3490-
current = root
3491-
for index, part in enumerate(parts):
3492-
is_last = index == len(parts) - 1
3493-
if is_last:
3494-
module_file = current / f"{part}.py"
3495-
if module_file.is_file():
3496-
return module_file
3497-
package_file = current / part / "__init__.py"
3498-
if package_file.is_file():
3499-
return package_file
3500-
else:
3501-
current = current / part
3502-
if not current.is_dir():
3503-
break
3595+
loaded_module = sys.modules.get(module_name)
3596+
loaded_spec = getattr(loaded_module, "__spec__", None)
3597+
if isinstance(loaded_spec, ModuleSpec) and isinstance(loaded_spec.origin, str):
3598+
if loaded_spec.origin.endswith(tuple(SOURCE_SUFFIXES)):
3599+
loaded_source_path = Path(loaded_spec.origin)
3600+
if loaded_source_path.is_file():
3601+
return loaded_source_path
3602+
if loaded_spec.origin not in {"built-in", "frozen"}:
3603+
return None
3604+
elif _untrusted_meta_path_finder_precedes(PathFinder, module_name) or _has_untrusted_path_hook():
3605+
return None
3606+
3607+
spec = _find_standard_filesystem_spec(module_name)
3608+
if spec is not None and isinstance(spec.origin, str) and spec.origin.endswith(tuple(SOURCE_SUFFIXES)):
3609+
source_path = Path(spec.origin)
3610+
if source_path.is_file():
3611+
return source_path
35043612
return None
35053613

35063614

packages/modelaudit-picklescan/tests/test_api.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from modelaudit_picklescan.call_graph import (
4141
CallGraphFinding,
4242
StartupHookWriteFinding,
43+
_call_graph_source_unavailable_reason,
4344
_CallGraphAnalysisLimitError,
4445
find_startup_hook_write_call_graphs,
4546
)
@@ -3317,9 +3318,20 @@ def test_scan_bytes_does_not_flag_dill_dump_as_dangerous() -> None:
33173318

33183319
report = scan_bytes(payload, source="dill-dump.pkl")
33193320

3320-
assert report.status == ScanStatus.COMPLETE
3321-
assert report.verdict == SafetyVerdict.CLEAN
33223321
assert report.findings == ()
3322+
source_reason = _call_graph_source_unavailable_reason("dill")
3323+
if source_reason is None:
3324+
assert report.status == ScanStatus.COMPLETE
3325+
assert report.verdict == SafetyVerdict.CLEAN
3326+
else:
3327+
assert report.status == ScanStatus.INCONCLUSIVE
3328+
assert report.verdict == SafetyVerdict.UNKNOWN
3329+
assert any(
3330+
notice.code == "call_graph_source_unavailable"
3331+
and notice.details.get("import_reference") == "dill.dump"
3332+
and notice.details.get("reason") == source_reason
3333+
for notice in report.notices
3334+
)
33233335

33243336

33253337
def test_scan_bytes_flags_dill_loads_as_dangerous() -> None:

0 commit comments

Comments
 (0)