4 ms·

My takeaway would be "using the C stack to parse JSON is probably not a good idea", not "accepting an unlimited depth structure by default is probably not a goo

My takeaway would be "using the C stack to parse JSON is probably not a good idea", not "accepting an unlimited depth structure by default is probably not a good idea". This limit is entirely artificial, and due only to the design of the parser. As the documentation you quote says, you should limit the size of JSON texts you accept. If you don't, then you are at risk of a memory exhaustion attack. But this is completely unrelated to the maximum depth you accept. If your parser doesn't use the C stack to parse recursive structures, then deeply nested payloads will parse just fine, without using an abnormal amount of memory.

chrismorgan 7y ago
I disagree with your conclusion. I would instead say that the combination of using the stack to parse JSON and allowing unlimited depth is the problem. In practice, almost no one ever deals with JSON structures even a hundred levels deep (I’d quite seriously guess it to be less than one in a million applications that desire it), so if you artificially cap it to protect against stack overflow you can then safely use the stack, thereby getting better performance by reducing the required allocations, while also protecting against one DoS attack vector.
- lovasoa 7y ago
  You never "artificially cap it to protect against stack overflow": Most libraries that cap it artificially cap it to a fixed value, that may or may not protect against stack overflow depending on how large the stack already is when you call the parser.
  - chrismorgan 7y ago
    These caps we’re talking about are normally a small fraction of the total stack size, and few applications run anywhere near the total stack size. I acknowledge your point (and had it in mind when I wrote my first comment), but maintain my position that this capping is protecting against the stack overflow DoS attack: if you’re able to get a stack overflow with your capped JSON parser, you were probably already able to get a stack overflow, because it’s not adding overly much to the stack. The problem that’s being protected against is user-controlled stack overflow—that’s what makes it a DoS attack.
jlokier 7y ago
> As the documentation you quote says, you should limit the size of JSON texts you accept. [...] But this is completely unrelated to the maximum depth you accept. I'm not sure that's right. The documentation points out that a deep structure returned from the parser can crash the interpreter, because freeing data also uses recursion. ("due to perl itself recursing deeply on croak to free the temporary".) Anyway, I found the 512 depth limit too much... When using coroutines in Perl, the stack is smaller and it crashes trying just to print a JSON object with depth 512. I hit the limit at about depth 150, with nothing else on the C stack, so in robust diagnostic code that uses the JSON formatter to log data structures, I limit the max depth to 20.

chrismorgan 7y ago

I disagree with your conclusion. I would instead say that the combination of using the stack to parse JSON and allowing unlimited depth is the problem. In practice, almost no one ever deals with JSON structures even a hundred levels deep (I’d quite seriously guess it to be less than one in a million applications that desire it), so if you artificially cap it to protect against stack overflow you can then safely use the stack, thereby getting better performance by reducing the required allocations, while also protecting against one DoS attack vector.

lovasoa 7y ago
You never "artificially cap it to protect against stack overflow": Most libraries that cap it artificially cap it to a fixed value, that may or may not protect against stack overflow depending on how large the stack already is when you call the parser.
- chrismorgan 7y ago
  These caps we’re talking about are normally a small fraction of the total stack size, and few applications run anywhere near the total stack size. I acknowledge your point (and had it in mind when I wrote my first comment), but maintain my position that this capping is protecting against the stack overflow DoS attack: if you’re able to get a stack overflow with your capped JSON parser, you were probably already able to get a stack overflow, because it’s not adding overly much to the stack. The problem that’s being protected against is user-controlled stack overflow—that’s what makes it a DoS attack.

jlokier 7y ago

> As the documentation you quote says, you should limit the size of JSON texts you accept. [...] But this is completely unrelated to the maximum depth you accept. I'm not sure that's right. The documentation points out that a deep structure returned from the parser can crash the interpreter, because freeing data also uses recursion. ("due to perl itself recursing deeply on croak to free the temporary".) Anyway, I found the 512 depth limit too much... When using coroutines in Perl, the stack is smaller and it crashes trying just to print a JSON object with depth 512. I hit the limit at about depth 150, with nothing else on the C stack, so in robust diagnostic code that uses the JSON formatter to log data structures, I limit the max depth to 20.