fickling icon indicating copy to clipboard operation
fickling copied to clipboard

can't create a safe python class

Open luccabb opened this issue 2 years ago • 5 comments

numpy_poc example has the following class as an example of an unsafe class:

...
class Test(object):
    def __init__(self):
        self.a = 1

    def __reduce__(self):
        # Runs the other PoC found in /examples
        return (os.system, ("python pytorch_poc.py",))
...

removing the unsafe __reduce__ method from the class is not enough to make it safe:

...
class Test(object):
    def __init__(self):
        self.a = 1
...
$ python example/numpy_poc.py
...
Is this is_likely_safe?
❌

Is this behavior expected?

luccabb avatar Sep 28 '23 22:09 luccabb

Thanks for raising this issue! This is not expected behavior, so we should investigate further to see what's causing this.

suhacker1 avatar Sep 29 '23 20:09 suhacker1

@luccabb Disclaimer: I'm not the maintainer. TLDR; I think any pickled object instance can't be classified as safe due to its opcode behavior.

When pickle any instance, the pickle uses NEWOBJ op to create an instance which is considered a "call".
https://github.com/trailofbits/fickling/blob/ce3ace17be880c90b4d0fac87baf9daca9e256a4/fickling/fickle.py#L1091 As a result, it won't pass the "not has_non_setstate_call" condition. https://github.com/trailofbits/fickling/blob/ce3ace17be880c90b4d0fac87baf9daca9e256a4/fickling/fickle.py#L647C9-L647C23

NEWOBJ is introduced in protocol 2. In protocol 1, the pickle uses REDUCE op so it won't pass neither. I'm not sure why pickle decides to use NEWOBJ when there is OBJ op.

Edit: nvm, OBJ and INST op are still a "call" and aren't safe. They are both also not supported in fickling.

Bankde avatar Oct 28 '23 15:10 Bankde

This issue is still present with the most recent updates, but there's now more context as to what is causing this. For this PoC, I saved detailed safety analysis results into test.json.

PoC:

from fickling.fickle import Pickled
import pickle
import fickling.analysis as analysis

class Test:
    def __init__(self):
        self.a = 1

payload = Test()

with open("test.pkl", "wb") as f:
    pickle.dump(payload, f)

with open("test.pkl", "rb") as f:
    fickled_payload = Pickled.load(f)

safety_results = analysis.check_safety(fickled_payload, json_output_path="test.json")

test.json:

{
    "severity": "LIKELY_UNSAFE",
    "analysis": "`from __main__ import Test` imports a Python module that is not a part of the standard library; this can execute arbitrary code and is inherently unsafe\nCall to `Test()` can execute arbitrary code and is inherently unsafe",
    "detailed_results": {
        "AnalysisResult": {
            "NonStandardImports": "from __main__ import Test",
            "OvertlyBadEval": "Test()"
        }
    }
}

suhacker1 avatar Dec 19 '23 19:12 suhacker1