But yeah, the article's example of an XCode segfault is a good example of both poor parsing logic and poor isolation of fault domains. If your json processing library can corrupt the whole process then something's wonky.
What I see a lot is initializing an object from a JSON parser which, when it receives malformed input, either halts execution outright or returns an empty object. In both cases you just want to make sure to handle the error appropriately per-language.
Again, the main offender here is PHP, which makes this issue surprisingly easy to get wrong. (And add "==" to your lint checks! 99% of the time coders mean "===", and this can lead to surprisingly severe security issues.)
Especially on the stack.
On of the test cases has 100000 [ and my own JSON parser crashed on them, because it parsed recursively and the stack overflowed. So now I have fixed it to parse iteratively.
It still crashes.
Big surprise. Turns out the array destructor is also recursive, because it deletes every element in the array. So the array destructor crashes at some random point, after the parser has worked correctly.
GP says "or catch the error", so they're probably using that term in this sense.
If the parser has some kind of partial parsing, a way to recover from errors or you are using a language in which returning explicit errors is the more common idiom, then you probably shouldn't throw an exception.
Throwing an exception when parsing fails is a near perfect example of where exceptions produce clarity of code.
That is to say, a parse function is usually a pure function that takes a string and returns some sort of object. As a pure function, it "answers a question". A parser answers the question: "What object does this string represent?"
When given bad input, the answer is not "NULL". It is not "-1". It is not a tuple of ("SUCCESS", NULL). The question itself has no answer. Exceptions enable code flows for questions without answers, and for procedures that can not be actualized.
Now, you can engineer it such that you change the question to "Give me a tuple that represents an error code and an object representing... etc." But then if your calling function can not answer the question it's meant to answer, you have to manually check the inner function result and return some code. With exceptions, stack unrolling comes for free, with zero code. Once you get used to expecting the secondary, exceptional code flow, you can read and design far cleaner code than you can otherwise.
The problem with exceptions is that they're not type-checked in most languages - you can throw whatever you want, and the type system doesn't reflect that, so there's no way to statically determine that all expected exceptions are properly handled.
They're (partially) type-checked in Java, but it's extremely annoying, because there's no ability to express that part of the type in a way that lets you write methods that are generic with respect to exceptions thrown by other code (e.g. there's no way to describe a method `f(T x)` such that it throws `E` plus everything that `x.g()` may throw, for any `T`).
This means that if you don't parse correctly, it would be None, if you do parse correctly it would be Some(T)
def load_data():
with open('some_file.json', 'r') as f_in:
data = parse_json_stream(f_in)
data['timestamp'] = some_date_fn() # Do something with the *definitely-valid* data on the next line.
return data
def parse_json_stream(io_stream):
# Some complex parser...
# at some point...
if next_char != ']':
raise JsonException('Expected "]" at line {}, column {}'. format(line, col))
# More parser code...
A benefit of exceptions here is that you don't have to check the result of "data = parse_json_stream(f_in)" to immediately work with the resulting data. The stack unwinds until it is in a function that can handle the exception.*edit: Code formatting.
Of course, these approaches are not isomorphic, but in Rust, the cost of explicitly checking an error value is typically very small thanks to algebraic data types, polymorphism and macros.
It sounds like the Web way of doing things doesn't have this tradition -- much like how it doesn't have static strict extensible type systems.
Dijkstra specifically calls out exceptions as structured control flow, and as being probably-acceptable, and not subject to his concerns.
More broadly, any argument that goes "Exceptions are an extension of GOTO, and therefore bad" has some questions to answer, given that nearly all control structures are implemented as an extension of GOTO.
As to your last sentence, I think you have it backwards. I speculate that of the code written in 2016, most of the code that did not use exceptions for control flow was Javascript async code. (There are of course other non-exception-using languages, but other than C they're mostly pretty fringe, and for good or for ill JS is so damn fecund).
Well put. Under the hood, every IF, ELSE, WHILE, SWITCH, FOR, and other decision point in structured code is implemented with at least one unconditional JMP.
He had reasons for saying what he said in the context of the times. Remove that temporal context and the why becomes more important than the what.
Joel Spolsky had much the same to say: http://www.joelonsoftware.com/items/2003/10/13.html . If I'm going to have to argue about this, I'd rather use Spolsky than Dijkstra as a starting point; what, if anything, do you see that's wrong in his article?
1. They are invisible in the source code
2. They create too many possible exit points
As a java developer I really don't find 1 to be a problem at all. Lots of people complain about checked exceptions, but they solve this problem. It's very easy to see which functions throw and which don't. I'd even argue that the invisibility that is there is a positive -- I usually don't want to think about the error case, but can easily trace the exceptional path if I do. I find that often functions need to go back up the stack to handle issues, in which case the exception control flow is exactly what I want.
Runtime exceptions, on the other hand, are invisible and can be an issue, but things like running out of memory, or overflowing the stack, well, I really don't want to have to write: public int add(int x, int y) throws OutOfMemoryException, StackOverflowException, ...
For 2, I find that they just create 1 extra exit point, which is back up the stack/to the catch handler. You could certainly abuse them to cause problems, but I personally don't find this to be an issue in practice.
I think that everyone agrees that unstructured flow of control is problematic, but checked exceptions do have structure, even if it's a bit of a loose one.
Some languages or libraries are explicitly designed to use exceptions for flow control some strongly discourage this. Apple's ObjC error handling guide for example contains this stricture but also calls out parsing as an example of when you should do it.
C.A.R 'null pointer' Hoare considered exceptions a blight on the earth that would lead to us accidentally nuking ourselves, so there's a spectrum of opinions available here.
Why?
(My thoughts on the matter: http://www.mcherm.com/reasons-why-my-code-style-is-wrong.htm... )
There is ongoing work to deprecate and remove it in favor of actual exceptions, which travel up the stack and are much easier to clean up after.
It is pretty rare to need to parse JSON yourself (what environment doesn't have that available?) but it isn't that difficult. It's a simple language.
As someone who has a web crawler, I'm painfully aware of how much syntactically incorrect HTML is out there. HTML5 has a whole section which standardizes how to parse bad HTML. That's just the syntax needed to parse it into a tree, without considering the semantics at all.
Actually, as someone who has written an HTML parser by following the HTML5 spec, I see it as the opposite: because every string of bytes essentially corresponds to some HTML tree, there are no special "invalid" edge cases to consider and everything is fully specified. That's the best situation, since bugs tend to arise at the edge cases, the boundaries between valid and invalid. But the HTML5 spec has no edge cases: the effect of any byte in any state of the parser has been specified.
HTML5 has a whole section which standardizes how to parse bad HTML.
In some ways, I think it's a bit of a moot point what is "bad HTML" if all parsers that conform to the standard parse it in the same way. The spec does mention certain points as being parse errors, but are they really errors after all, if the behaviour is fully specified (and isn't "give up")? In fact, disregarding whether some states are parse errors actually simplifies the spec greatly because many of the states can be coalesced, and for something like a browser or crawler, it's completely irrelevant whether any of these "parse errors" actually occurred during the parsing. One example that comes to mind is spaces after quoted attribute values; a="b" c="d" and a="b"c="d" are parsed identically, except the latter is supposedly a parse error. Yet both are unambiguous.
(for the record, they are the empty string, and any string that is an ASCII case-insensitive match for the string "transparent")
In theory, it's not supposed to be "that difficult". But in practice, according to the linked-to article, due to all rot and general clusterfuck-ery in the various competing specifications, apparently it is.
Or do you really think you could wrap your head all around those banana peels, and put together a robust, production-ready parser in a weekend?
I wouldn't want my own JSON parser out on the web, but if I needed to get JSON from $known_environment to $my_service, I'd feel safe enough with a parser I wrote.
Taking it the other way, we can also ask "how can you optimize the parser to be as simple as possible, so that everything is well-specified and nobody can eff it up, while still preserving the structure that JSON gives you?" I tried to experiment with that about five years ago and came up with [1], but it shows a nasty cost differential between "human-readable" and "easy to parse." For example, the easiest string type to parse is a netstring, and this means automatic consistent handling of embedded nulls and what-have-you... but when those unreadable characters aren't escaped then you inherently have trouble reading/writing the file with a text editor. Similarly the easiest type of float is just to take the 64 bits and either dump them directly or as hex... but either way you don't have the ability to properly edit them with the text editor. Etc.
But I am finding that the central problem I'm having with JSON and XML is that it's harder to find (and harder to control!) streaming parsers, so one thing I'm thinking about for the future is that formats that I use will probably need to be streaming from the top-level.[2] So if anyone's reading this and designing stuff, probably even more important than making the parser obviously correct is making it obviously streaming.
[1] https://github.com/drostie/bsencode is based on having an easy-to-parse "outer language" of s-expressions, symbols, and netstrings, followed by an interpretation step where e.g. (float 8:01234567) is evaluated to be the corresponding float.
[2] More recently I've had a lot of success in dealing with more-parallel things to have streamability; for example if you remove whitespace from JSON then [date][tab][process-id][tab][json][newline] is a nice sort of TSV that gets really useful for a workflow of "append what you're about to do to the journal, then do it, then append back that it's done" and so forth; when a human technician needs to go back through the logs they have what they need to narrow down (a) when something went wrong, (b) what else was on that process when it was going wrong, (c) what did it do and what was it trying to do? ... you can of course do all this in JSON but then you need a streaming JSON parser, whereas everyone can do the line-buffering of "buffer a line, split the next chunk by newlines, prepend the first line with the buffer, then save the last line to the buffer, then emit the buffered lines and wait on the next chunk."
Autocad LISP is the one I ran into. At least it didn't 4 years ago. I'm sure there are other niche cases.
Although I'll admit I punted and wrote a trivial 'parser' that was only able to read and write the particular JSON I was dealing with in that project.
Note it's only two years old, so wouldn't have helped last time ;-)
That, coupled with the fact that it is still so easy to get it wrong and to introduce security issues is exactly what should peak your attention to the seriousness of the subject. Building any parser is fraught with risk, it is super easy to get it subtly and horribly wrong.
It ought to be feared, if interoperability is involved. The problem isn't that you might introduce security issues. The problem is usually that you introduce very subtle deviations to the spec that everyone else implemented correctly, and as a result, sometimes your input and/or output do not work with other stuff out there.
Okay so it's more terrifying in C than most other things, but still, it's terrifying. Runaway memory consumption, weird Unicode behaviour etc. etc. etc. It's easy to think you don't have to worry about Unicode because your language's string types will handle it for you - but what do they do if the input contains invalid codepoints? You're writing a parser, you need to know - and possibly override it if that behaviour conflicts with the spec.
Horrible business. Definitely not my favourite job.
You can get an implementation working with a fairly high level of confidence that it's right.
If it's not fast enough, make a pretty printer for your AST. Then do a CPS transform (by hand) on your library and parser, so you can make the stack explicit. Make sure the transformed version pretty prints exactly the same way.
Then make a third version that prints out the code that should run when parsing a document, rather than doing the parsing directly. You'll get a big case switch for each grammar you want to parse. Your pretty printer will help you find many bugs.
It's a pretty achievable path to get your grammar correct, and then get a specialized parser for it.
[1] http://winterbe.com/posts/2014/04/05/java8-nashorn-tutorial/
A modern JS implementation should also have a JSON parser
Or the other way around. Think about something like MUMPS.
First, parsing JSON is trivial compared to other parsing tasks. There are no cycles as in YAML or other serializers, it's trivial forward scanning, without any need to tokenize or backtracking.
Second, JSON is one of the simplest formats out there, and due its simplicity also its most secure. It has some quirks and some edge cases are not well-defined. But with those problems you can always check against your local javascript implementation and the spec, just as OP did.
I know very few JSON parsers which actually crash on illegal input. There are some broken ones, but there are much more broken and insecure by default YAML or BSON parsers or language serializers, like pickle, serialize, Storable, ...
Parsing JSON is not a minefield, parsing JSON is trivial.
Takeaway: Favor JSON over any other serialization format, even if there are some ill-defined edgecases, comments are disallowed and the specs are not completely sound. The YAML and XML specs are much worse, their libraries horrible and bloated.
JSON is the only secure by default serializer. It doesn't allow objects nor code, it doesn't allow cyclic data, no external data, it's trivial, it's fast.
Having summarized that, I'm wondering why OP didn't include my JSON parser in his list, Cpanel::JSON::XS, which is the default fast JSON serializer for perl, is the fastest of all those parsers overall, and is the only one which does pass all these tests. Even more than the new one which OP wrote for this overview STJSON. The only remaining Cpanel::JSON::XS problem is to decode BOM of UTF16 and UTF32. Currently it throws an error. But there are not even tests for that. I added some.
Regarding security: https://metacpan.org/pod/Cpanel::JSON::XS#SECURITY-CONSIDERA...
This attitude really pisses me off. Get off your high horse.
I have actually written an HTML parser, and it was not so difficult because the HTML spec specifies everything down to the level of tokenising individual characters.
He's not talking about rolling your own for fun or learning reasons which anyone should feel free to do.
And quit using war metaphors to feel macho about your work. What we do in no way resembles battle. Neither are most libraries tested to the degree you are trying to suggest. Neither is common usage in several projects a reliable source if testing.
When people say things like "don't reinvent the wheel" or "only for yourself, not for your employer", what they mean is "I know I can't, and who do you think you are thinking you're better than me. Get back in your place."
And I don't know about you but these days hosting a public facing internet service is increasingly like a battle DDOS, Data Loss, State sponsored APT's. I certainly don't feel macho about my work. I don't author many libraries that are used by a public facing service on the internet so I have nothing to beat my chest about. I do however use libraries that have been written by someone else and had the bugs shaken out by years of continuous use in a hostile environment. It's not a perfect guarantee but it's certainly better than something I wrote this month.
In most cases a library that has been in continuous use for several years on large public facing services will have been much more tested than anything you might write. Not because you aren't skilled enough but because the edge cases are unbounded and you can't think of everything.
Parsing everything is NOT a minefield. We have BNF's parser theory etc for that. Lots of languages have clear unambigious definitions... Json clearly not. ITs a disgrace for the software engineering community.
Been there done that. (The medical attention, I mean.) Worked just fine. The article makes it sound extremely difficult, but 100% of the article is about edge cases that rarely happen with normal encoders and can often be ignored (e.g. who cares if you escape your tab character?).
> consider what happens when the code opens a session, sets the session username, then parses some input JSON before the password is evaluated
Edit: I responded more elaborately on the unlikelihood of this, but honestly, I can't come up with a single conceivable scenario. How would you decode part of the JSON and only parse the password bit later?
The scenario is typically username-password-parameters passed as three variables via POST. The offending developer parses all three variables up front for simplicity:
The session user is created from the POST username, the POST password is input sanitized and put in a variable, then the parameters are parsed as JSON. The attacker POSTs a valid user, a valid (but incorrect) password, and malformed JSON.
The login code reads the variables from POST, opens the session, and dies on the JSON decode.
Hint: this bug exists in the wild on a massively deployed framework. I'm working up a responsible disclosure on that one now.
Alright you win. I'm curious to know which one it was after it's fixed!
I really didn't expect this to exist...
That's the part I don't get. Once the code dies it's done. What exactly can you do now, if no code is even running?
1. Parse user.
2. Parse password.
3. Create session object from user and password.
4. Parse JSON. (Crashes here.)
5. Update session and do some other security stuff.
The problem is that they're creating a persistent session object ahead of when the JSON parameters are being decoded, which leaves a (partially initialized, persisting-outside-of-function) session object without having gone through the full authentication flow.
The problem is that they're creating permanent objects ahead of a full validation on the data necessary to actually properly initialize the object.
(I think the OP may have been a little vague because that's probably specific enough to identify the vuln with a scanner and a hunch.)
I don't do PHP. It just sounds incredible to me that so little isolation seems to exist.
I look forward to the disclosure to understand more about this - and check that the frameworks I use don't have this problem.
I've found that "edge cases that rarely happen" are typically 90% of my major headaches.
Yeah, that seems like a problem regardless of whether or not you're parsing JSON.
Not saying it's common or likely in this scenario, but there are streaming JSON parsers (like SAX for XML)
As someone who has written his own JSON parser, I must concur. Ahh - are there any doctors here...?
In my defense - I was porting a codebase to a new platform, and needed to replace the existing JSON 'parser'. You see, it was:
- Single-platform
- Proprietary
- Little more than a tokenizer with idiosyncrasies and other warts
Why was it chosen in the first place? Well, it was available as part of the system on the original platform. Not that I would've made the same choice myself. We had wrappers around it - but they didn't really abstract it away in any meaningful manner. So all of it's idiosyncrasies had leaked into all the code that used the wrappers. In the interests of breaking as little existing code as possible, I wrote a bunch of unit tests, and rewrote the wrapper in terms of my own hand rolled tokenizer. Later - either after the port, or as a side project during the port to help out a coworker (I forget) - I added some saner, higher level, easier to use, less idiosyncratic interfaces - basically allowing us to deprecate the old interface and clean it up at our leisure. This basically left us with a full blown parser - and it was all my fault.> Takeaways: Don't parse JSON yourself, and don't let calls to the parsing functions fail silently.
I'd add to this: Fuzz your formats. All of them. Even those that don't receive malicious data will receive corrupt data.
Many of the same problems also affect e.g. binary formats. And just because you've parsed valid JSON doesn't mean you're safe. I've spent a decent amount of time using e.g. SDL MiniFuzz - fixing invalid enum values, unchecked array indicies, huge array allocations causing OOMs, bad hashmap keys, the works. The OOM case is particularly nasty - you may successfully parse your entire input (because 1.9GB arrays weren't quite enough to OOM your program during parsing), and then later randomly crash anywhere else because you're not handling OOMs throughout the rest of your program. I cap the maximum my parser will allocate to some multiple of the original input, and cap the original input to "something reasonable" (1MB is massive overkill for most of my JSON API needs, for example, so I use it as a default.)
I had the advantage that I only needed to use it for rapid prototype projects, and that I could count on all of the data from a single source being the same "shape" (only the scalar values changed, never the keys or objects).
Not following the RFC helped greatly, as I just dgaf about things like trailing commas.
The biggest "gotcha" for my first implementation was a) temporary object allocation and b) recursion depth. The third prototype to use the parser needed to parse, I think it was, a ten thousand line JSON file? The garbage collection on all the temporary objects (mostly strings) meant that version took ~30 seconds. I refactored to be non-recursive and re-use string variable wherever possible, and it dropped down to seconds.
There was a moment in writing it that I thought I would have an actually legitimate use for GOTO (a switch case that was mostly a duplicate of another case), but that turned out not to be the case :/
Well, I also wanted a YAML parser, and had the weird need to handle JSON inside XML (oh, industrial sector, and your SOAP), and to not care about things like trailing commas, and then at some point to deal with weirdly large amounts of data.
Each iteration only took a couple of days, and I learned a bunch about tokenizing things, and then dealing with token streams.
The === issue is in JS as well.
Secondly, assume that parsing your input will crash, so catch the error and have your application fail gracefully.
This is the number one security issue I encounter in "security audited" PHP. (The second being the "==" vs. "===" debacle that is PHP comparison.)
As one example, consider what happens when the code opens a session, sets the session username, then parses some input JSON before the password is evaluated. Crashing the script at the json_decode() fails with the session open, so the attacker can log in as anyone.
Third, parsing everything is a minefield, including HTML. We as a community invest a lot of collective effort in improving those parsers, but this article does serve as a useful reminder of a lot of the infrastructure we take for granted.
Takeaways: Don't parse JSON yourself, and don't let calls to the parsing functions fail silently.