diff --git a/fickling/fickle.py b/fickling/fickle.py index 2b2dd56..b92f444 100644 --- a/fickling/fickle.py +++ b/fickling/fickle.py @@ -647,7 +647,11 @@ def _process_import(self, node: ast.Import | ast.ImportFrom): and node.module is not None and is_std_module(node.module) ): - self.likely_safe_imports |= {name.name for name in node.names} + self.likely_safe_imports |= { + n.name + for n in node.names + if not any(c in UNSAFE_IMPORTS for c in n.name.split(".")) + } def visit_Import(self, node: ast.Import): self._process_import(node) @@ -1122,20 +1126,20 @@ def is_likely_safe(self): ) def unsafe_imports(self) -> Iterator[ast.Import | ast.ImportFrom]: + def is_unsafe(dotted: str) -> bool: + return any(c in UNSAFE_IMPORTS for c in dotted.split(".")) + for node in self.properties.imports: if isinstance(node, ast.ImportFrom): - if node.module and any( - component in UNSAFE_IMPORTS for component in node.module.split(".") - ): + if node.module and is_unsafe(node.module): + yield node + elif any(is_unsafe(n.name) for n in node.names): yield node elif "eval" in (n.name for n in node.names): yield node else: # ast.Import: check if any imported module is unsafe - if any( - any(component in UNSAFE_IMPORTS for component in alias.name.split(".")) - for alias in node.names - ): + if any(is_unsafe(alias.name) for alias in node.names): yield node elif "eval" in (alias.name for alias in node.names): yield node diff --git a/test/test_bypasses.py b/test/test_bypasses.py index e24e2d4..4a6020a 100644 --- a/test/test_bypasses.py +++ b/test/test_bypasses.py @@ -684,6 +684,26 @@ def test_missing_osx_support(self): "from _osx_support import _find_build_tool", ) + # https://github.com/trailofbits/fickling/security/advisories/GHSA-5j3x-jp52-966f + def test_dotted_attr_via_stdlib_module(self): + pickled = Pickled( + [ + op.Proto.create(4), + op.ShortBinUnicode("pathlib"), + op.ShortBinUnicode("os.system"), + op.StackGlobal(), + op.EmptyTuple(), + op.Reduce(), + op.Stop(), + ] + ) + res = check_safety(pickled) + self.assertGreater(res.severity, Severity.LIKELY_SAFE) + self.assertEqual( + res.detailed_results()["AnalysisResult"].get("UnsafeImports"), + "from pathlib import os.system", + ) + # https://github.com/trailofbits/fickling/security/advisories/GHSA-cffv-grgg-g429 def test_ml_allowlist_not_shadowed_by_unsafe_imports_ml(self): """MLAllowlist must flag imports outside ML_ALLOWLIST even when another