From ec33e94622b527ef2b8de2edb36acacccce92deb Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Fri, 18 Feb 2022 17:55:02 +0100 Subject: [PATCH 1/7] feat(Pickled): add vetted imports and functions properties --- fickling/pickle.py | 49 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/fickling/pickle.py b/fickling/pickle.py index 62e5d4b..96340f1 100644 --- a/fickling/pickle.py +++ b/fickling/pickle.py @@ -246,6 +246,8 @@ def __init__(self, opcodes: Iterable[Opcode]): self._opcodes: List[Opcode] = list(opcodes) self._ast: Optional[ast.Module] = None self._properties: Optional[ASTProperties] = None + self._vetted_dependencies: Optional[List[str]] = None + self._vetted_calls: Optional[List[str]] = None def __len__(self) -> int: return len(self._opcodes) @@ -404,11 +406,37 @@ def has_import(self) -> bool: """Checks whether unpickling would cause an import to be run""" return bool(self.properties.imports) + @property + def has_unvetted_import(self) -> bool: + if self._vetted_dependencies is None: + raise Exception("Cannot call has_unvetted_import when vetted_dependencies is not set") + unvetted_import = False + for node in self.properties.imports: + module_path = node.module.split(".") + if module_path[0] not in self._vetted_dependencies: + unvetted_import = True + break + return unvetted_import + @property def has_call(self) -> bool: """Checks whether unpickling would cause a function call""" return bool(self.properties.calls) + @property + def has_unvetted_calls(self) -> bool: + if self._vetted_calls is None: + raise Exception("Cannot call has_unvetted_functions when vetted_functions is not set") + unvetted_call = False + for call in self.properties.non_setstate_calls: + function = call.func.id + if function == "_reconstruct": + function = call.args[0].id + if function not in self._vetted_calls: + unvetted_call = True + break + return unvetted_call + @property def has_non_setstate_call(self) -> bool: """Checks whether unpickling would cause a call to a function other than object.__setstate__""" @@ -417,7 +445,10 @@ def has_non_setstate_call(self) -> bool: @property def is_likely_safe(self) -> bool: # `self.has_call` is probably safe as long as `not self.has_import` - return not self.has_import and not self.has_non_setstate_call + if self._vetted_dependencies is None or self._vetted_calls is None: + return not self.has_import and not self.has_non_setstate_call + else: + return not self.has_unvetted_import and not self.has_unvetted_calls def unsafe_imports(self) -> Iterator[Union[ast.Import, ast.ImportFrom]]: for node in self.properties.imports: @@ -437,6 +468,22 @@ def ast(self) -> ast.Module: self._ast = Interpreter.interpret(self) return self._ast + @property + def vetted_dependencies(self) -> Optional[List[str]]: + return self._vetted_dependencies + + @vetted_dependencies.setter + def vetted_dependencies(self, dependencies: List[str]): + self._vetted_dependencies = dependencies + + @property + def vetted_calls(self) -> Optional[List[str]]: + return self._vetted_calls + + @vetted_calls.setter + def vetted_calls(self, functions: List[str]): + self._vetted_calls = functions + class Stack(GenericSequence, Generic[T]): def __init__(self, initial_value: Iterable[T] = ()): From 769bea85b5d0c3ae3dffe9eb8823de1dfbc227ae Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Fri, 18 Feb 2022 17:55:11 +0100 Subject: [PATCH 2/7] feat(example): add vetted ex --- example/vetted.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 example/vetted.py diff --git a/example/vetted.py b/example/vetted.py new file mode 100644 index 0000000..96cf786 --- /dev/null +++ b/example/vetted.py @@ -0,0 +1,15 @@ +from fickling.pickle import Pickled +import pickle +import numpy as np + + +arr = np.array([1,2,3]) +fickled = Pickled.load(pickle.dumps(arr)) +fickled.vetted_dependencies = ["numpy"] +fickled.vetted_calls = ["ndarray", "dtype"] + +is_safe = fickled.is_likely_safe +if is_safe: + print("✅") +else: + print("❌") From b2d272da553624753cb48893bc4b7d33c0009aaf Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Fri, 18 Feb 2022 17:55:20 +0100 Subject: [PATCH 3/7] feat: add __pycache__ to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..c18dd8d --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +__pycache__/ From bb18d5e1ee21cefd2082034293df6a0925009550 Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Tue, 1 Mar 2022 11:34:44 +0100 Subject: [PATCH 4/7] fix(Pickled): replace Exception w/ ValueError --- example/vetted.py | 2 +- fickling/pickle.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/example/vetted.py b/example/vetted.py index 96cf786..ec78677 100644 --- a/example/vetted.py +++ b/example/vetted.py @@ -3,7 +3,7 @@ import numpy as np -arr = np.array([1,2,3]) +arr = np.array([1, 2, 3]) fickled = Pickled.load(pickle.dumps(arr)) fickled.vetted_dependencies = ["numpy"] fickled.vetted_calls = ["ndarray", "dtype"] diff --git a/fickling/pickle.py b/fickling/pickle.py index 96340f1..b355679 100644 --- a/fickling/pickle.py +++ b/fickling/pickle.py @@ -409,7 +409,7 @@ def has_import(self) -> bool: @property def has_unvetted_import(self) -> bool: if self._vetted_dependencies is None: - raise Exception("Cannot call has_unvetted_import when vetted_dependencies is not set") + raise ValueError("Cannot call has_unvetted_import when vetted_dependencies is not set") unvetted_import = False for node in self.properties.imports: module_path = node.module.split(".") @@ -426,7 +426,7 @@ def has_call(self) -> bool: @property def has_unvetted_calls(self) -> bool: if self._vetted_calls is None: - raise Exception("Cannot call has_unvetted_functions when vetted_functions is not set") + raise ValueError("Cannot call has_unvetted_functions when vetted_functions is not set") unvetted_call = False for call in self.properties.non_setstate_calls: function = call.func.id From c47e1ae51cb35693d333e3af5af490fa5c7e38f5 Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Mon, 7 Mar 2022 15:17:09 +0100 Subject: [PATCH 5/7] refacto(Pickled): unvetted dependencies * I am for now not assuming anything about unvetted dependencies but am just providing a helper function to find them --- example/unvetted_dependencies.py | 25 ++++++++++++++++ example/vetted.py | 15 ---------- fickling/pickle.py | 51 +++++++++++--------------------- 3 files changed, 43 insertions(+), 48 deletions(-) create mode 100644 example/unvetted_dependencies.py delete mode 100644 example/vetted.py diff --git a/example/unvetted_dependencies.py b/example/unvetted_dependencies.py new file mode 100644 index 0000000..ad94f9d --- /dev/null +++ b/example/unvetted_dependencies.py @@ -0,0 +1,25 @@ +from fickling.pickle import Pickled +import numpy as np +import pickle +from sklearn import svm, datasets + +def check_vetted(p: Pickled): + if p.has_unvetted_dependency: + print(f"unvetted deps : {p.unvetted_dependencies}") + +# sklearn +clf = svm.SVC() +X, y = datasets.load_iris(return_X_y=True) +clf.fit(X, y) +s = pickle.dumps(clf) + +p = Pickled.load(s) +p.vetted_dependencies = ["numpy.ndarray", "sklearn.svm._classes.SVC", "numpy.core.multiarray._reconstruct", "numpy.dtype", "numpy.core.multiarray.scalar"] +check_vetted(p) + +# numpy +arr = np.ndarray([1, 2, 3]) +p = Pickled.load(pickle.dumps(arr)) +p.vetted_dependencies = ["numpy.ndarray"] +check_vetted(p) + diff --git a/example/vetted.py b/example/vetted.py deleted file mode 100644 index ec78677..0000000 --- a/example/vetted.py +++ /dev/null @@ -1,15 +0,0 @@ -from fickling.pickle import Pickled -import pickle -import numpy as np - - -arr = np.array([1, 2, 3]) -fickled = Pickled.load(pickle.dumps(arr)) -fickled.vetted_dependencies = ["numpy"] -fickled.vetted_calls = ["ndarray", "dtype"] - -is_safe = fickled.is_likely_safe -if is_safe: - print("✅") -else: - print("❌") diff --git a/fickling/pickle.py b/fickling/pickle.py index b355679..fb5ea72 100644 --- a/fickling/pickle.py +++ b/fickling/pickle.py @@ -247,7 +247,7 @@ def __init__(self, opcodes: Iterable[Opcode]): self._ast: Optional[ast.Module] = None self._properties: Optional[ASTProperties] = None self._vetted_dependencies: Optional[List[str]] = None - self._vetted_calls: Optional[List[str]] = None + self._unvetted_dependencies: Optional[List[str]] = None def __len__(self) -> int: return len(self._opcodes) @@ -407,36 +407,28 @@ def has_import(self) -> bool: return bool(self.properties.imports) @property - def has_unvetted_import(self) -> bool: + def has_unvetted_dependency(self) -> bool: if self._vetted_dependencies is None: raise ValueError("Cannot call has_unvetted_import when vetted_dependencies is not set") - unvetted_import = False - for node in self.properties.imports: - module_path = node.module.split(".") - if module_path[0] not in self._vetted_dependencies: - unvetted_import = True - break - return unvetted_import + if self._unvetted_dependencies is None: + self._unvetted_dependencies = [] + for st in self.ast.body: + if isinstance(st, ast.ImportFrom): + for imp in st.names: + import_path = f"{st.module}.{imp.name}" + if import_path not in self._vetted_dependencies: + self._unvetted_dependencies.append(import_path) + return len(self._unvetted_dependencies) > 0 + elif len(self._unvetted_dependencies) > 0: + return True + else: + return False @property def has_call(self) -> bool: """Checks whether unpickling would cause a function call""" return bool(self.properties.calls) - @property - def has_unvetted_calls(self) -> bool: - if self._vetted_calls is None: - raise ValueError("Cannot call has_unvetted_functions when vetted_functions is not set") - unvetted_call = False - for call in self.properties.non_setstate_calls: - function = call.func.id - if function == "_reconstruct": - function = call.args[0].id - if function not in self._vetted_calls: - unvetted_call = True - break - return unvetted_call - @property def has_non_setstate_call(self) -> bool: """Checks whether unpickling would cause a call to a function other than object.__setstate__""" @@ -445,10 +437,7 @@ def has_non_setstate_call(self) -> bool: @property def is_likely_safe(self) -> bool: # `self.has_call` is probably safe as long as `not self.has_import` - if self._vetted_dependencies is None or self._vetted_calls is None: - return not self.has_import and not self.has_non_setstate_call - else: - return not self.has_unvetted_import and not self.has_unvetted_calls + return not self.has_import and not self.has_non_setstate_call def unsafe_imports(self) -> Iterator[Union[ast.Import, ast.ImportFrom]]: for node in self.properties.imports: @@ -477,12 +466,8 @@ def vetted_dependencies(self, dependencies: List[str]): self._vetted_dependencies = dependencies @property - def vetted_calls(self) -> Optional[List[str]]: - return self._vetted_calls - - @vetted_calls.setter - def vetted_calls(self, functions: List[str]): - self._vetted_calls = functions + def unvetted_dependencies(self) -> Optional[List[str]]: + return self._unvetted_dependencies class Stack(GenericSequence, Generic[T]): From dd7c90423df831a202c79b2652a64a55f9623429 Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Mon, 7 Mar 2022 15:21:21 +0100 Subject: [PATCH 6/7] feat(example): add is likely safe check in unvetted deps --- example/unvetted_dependencies.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/example/unvetted_dependencies.py b/example/unvetted_dependencies.py index ad94f9d..4882517 100644 --- a/example/unvetted_dependencies.py +++ b/example/unvetted_dependencies.py @@ -16,10 +16,18 @@ def check_vetted(p: Pickled): p = Pickled.load(s) p.vetted_dependencies = ["numpy.ndarray", "sklearn.svm._classes.SVC", "numpy.core.multiarray._reconstruct", "numpy.dtype", "numpy.core.multiarray.scalar"] check_vetted(p) +if p.is_likely_safe: + print("✅") +else: + print("❌") # numpy arr = np.ndarray([1, 2, 3]) p = Pickled.load(pickle.dumps(arr)) p.vetted_dependencies = ["numpy.ndarray"] check_vetted(p) +if p.is_likely_safe: + print("✅") +else: + print("❌") From 5adbba6f3fce9c1f42f77dcc1d8c86ea8b05ddd0 Mon Sep 17 00:00:00 2001 From: Luc Georges Date: Mon, 7 Mar 2022 15:30:14 +0100 Subject: [PATCH 7/7] feat(doc): add a line of doc about the feature --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 451a6d1..0d1e7fa 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,12 @@ You can also safely trace the execution of the Pickle virtual machine without ex Finally, you can inject arbitrary Python code that will be run on unpickling into an existing pickle file with the `--inject` option. +### Unvetted Dependencies + +You can check for unvetted dependencies in `fickling` by giving the `Pickled` class a list of "vetted" function calls from given modules. + +See [example/unvetted_dependencies.py](example/unvetted_dependencies.py) + ## License This utility was developed by [Trail of Bits](https://www.trailofbits.com/).