"aggressive" YAML anchors causing org.yaml.snakeyaml.Yaml.load() to hang

[fran fran]

Hi,

when passing a "YAML Bomb"-like content (I suspect possibly also with "simpler" content with anchors) to org.yaml.snakeyaml.Yaml.load(), the call hangs with high CPU usage.

e.g.

String content = readAsString("malicious.yaml");
org.yaml.snakeyaml.Yaml yaml = new org.yaml.snakeyaml.Yaml(new SafeConstructor());
yaml.load(content);

an example is provided here

Tested with Java 8, version 1.23

[Andrey Somov]

What is the issue ? What stops you from counting the & and * chars to reject the document ?

[fran fran]

Hi Andrey, thanks for your answer; as for your questions, let me clarify:

we are using snakeyaml in swagger-parser, indirectly via Jackson and also "directly" - to provide YAML anchors support, not available via Jackson - e.g. in this code.

As mentioned that line hangs for let's call it malicious input as the above, or also e.g. a file like (although with different behaviour):

a: &a ["lol","lol","lol","lol","lol","lol","lol","lol","lol"]
b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]
c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]
d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]
e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]
f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]
g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]
h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]
i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]

In my opinion counting & and * chars limiting somehow the number of anchors and aliases used would be difficult to implement and kind of out of scope . For example the input above has 9 anchors and 72 aliases, difficult to say if they are "too much" and also understanding the "nature" of such anchors, meaning for example if they are nested/recursive or they are "flat" (e.g. several "valid" first level elements each with "inoffensive" anchors/aliases).

With this I mean that possibly this kind of validation/protection would need yaml parsing capabilities, and therefore would fit more naturally in scope of yaml parser;

We are currently trying to overcome the issue by extending SafeConstructor for how much possible and perform some check in overridden methods (see this PR), but this doesn't seem to cover all cases. I am also trying to get a deeper understanding of snakeyaml behaviour, specifically for anchor handling, but I am not yet there..

Basically therefore the questions would be if you feel that such a "protection" could be part of the snakeyaml code, and/or if you have any alternative suggestion to handle such cases (I don't know, maybe some existing api/functionality allowing to check beforehand the wannabe result or something)

Thanks

[Andrey Somov]

The usecase you mentioned works perfectly. It is a part of the test suite: https://bitbucket.org/asomov/snakeyaml/src/default/src/test/java/org/yaml/snakeyaml/issues/issue377/ReferencesTest.java

The problem is that a key might be a map.

If your solution is general, there is no problem to make it a part of SnakeYAML

[fran fran]

My bad about the last use case, sorry, it indeed works ok, and fails in Jackson.

I am looking into the original issue to see if I can come up with a PR, thanks for your help

[Andrey Somov]

Do you have a clone of SnakeYAML at bitbucket ? I can give a pull request with a possible solution to consider

[fran fran]

Thanks, PR is welcome! here is my fork https://bitbucket.org/frantumagmail/snakeyaml

[Andrey Somov]

configure recursive keys

[Andrey Somov]

Sorry, unfortunately in order to create a pull request I have to implement it in the main repo (and this is exactly what I do not want). I have attach the implementation. Thai is similar to what is done for snakeyaml-engine: https://bitbucket.org/asomov/snakeyaml-engine/commits/7573a8b4d551fc84521f4ac1234a361bfbc96698

Feel free to review it and cover with tests.

[Andrey Somov]

No activity for half a year