14 ms·

Well, first and most obviously, if you are thinking of rolling your own JSON parser, stop and seek medical attention. Secondly, assume that parsing your input

• lucb1e 10y ago
  > Well, first and most obviously, if you are thinking of rolling your own JSON parser, stop and seek medical attention. 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?
  • wtetzner 10y ago
    > Setting the session flag for "this user is logged in" before checking (or even decoding!) the password seems rather backwards to me. Yeah, that seems like a problem regardless of whether or not you're parsing JSON.
    • rch 10y ago
      Probably a symptom of the PHP multiverse: anything that can happen, has happened.
      • oblio 10y ago
        And will continue to happen in a Wordpress setup somewhere.
  • tlrobinson 10y ago
    > How would you decode part of the JSON and only parse the password bit later? Not saying it's common or likely in this scenario, but there are streaming JSON parsers (like SAX for XML)
  • nulagrithom 10y ago
    > edge cases that rarely happen with normal encoders and can often be ignored I've found that "edge cases that rarely happen" are typically 90% of my major headaches.
    • lucb1e 10y ago
      Certainly, edge cases are a source of pain. Luckily I mentioned that even if you ignore most of these edge cases (as a parser) the data will come out just fine.
  • s_q_b 10y ago
    Yeah, the whole idea is a terrible practice, but it is used in industry surprisingly often. People think that PHP's session lock will save them, seeing the potential race condition but not the decode failure. 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.
    • lucb1e 10y ago
      > Hint: this bug exists in the wild on a massively deployed framework. I'm working up a 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...
    • ars 10y ago
      > and dies on the JSON decode 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?
      • SomeStupidPoint 10y ago
        The steps are the following: 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.)
• mikeash 10y ago
  A parser should never crash on bad input. If it does, that's a serious bug that needs immediate attention, since that's at least a DoS vulnerability and quite likely something that could result in remote code execution. You definitely need to assume that the parser could fail, but that's different. Unless you're using "crash" in some way I'm not familiar with?
  • PaulHoule 10y ago
    Recursion kills.
  • herge 10y ago
    What about raise an exception?
    • mikeash 10y ago
      What about it?
    • masklinn 10y ago
      That qualifies as error reporting in most languages using exceptions.
    • marxidad 10y ago
      Exceptions should only be used for exceptional cases. For a parser, bad input should be expected.
      • esrauch 10y ago
        You say that as fact but you must know it is a matter of opinion: I would say you should match the language idioms. For example, Python iterators and generators work by raising/throwing when there are no more items: it is fully expected and will always happen when you write a for-in loop.
        • iopq 10y ago
• inimino 10y ago
  Parsing HTML is literally orders of magnitude more complex. 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.
  • s_q_b 10y ago
    Parsing JSON is indeed several orders of magnitude less complex than parsing HTML. In both cases there are excellent parsing libraries, but it would be very unwise to create your own HTML parser for production use, or to use any parser that hasn't seen some serious scrutiny.
    • Animats 10y ago
      JSON at least has the concept of "invalid JSON". That's a big step forward. A JSON parser, like an XML parser, can say "Syntax error - rejected." There's no such thing as "invalid HTML". For that reason, parsing HTML is a huge pain. 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.
      • userbinator 10y ago
        There's no such thing as "invalid HTML". For that reason, parsing HTML is a huge pain. 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.
• edoceo 10y ago
  The article is about how the parsers behave differently. No suggestions of writing from scratch. The === issue is in JS as well.
• moron4hire 10y ago
  >> Well, first and most obviously, if you are thinking of rolling your own JSON parser, stop and seek medical attention This attitude really pisses me off. Get off your high horse.
  • userbinator 10y ago
    No kidding. If everyone thought that way, the only JSON parser to exist and which everyone uses would be barely working, because everyone would be put off from trying to write something better. 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.
  • zaphar 10y ago
    The implicit subtext here is "for production". It's an excellent rule of thumb since it's highly likely that one already exists that is already battle tested. It's better to use that one. It's not a high horse it's just good horse sense. He's not talking about rolling your own for fun or learning reasons which anyone should feel free to do.
    • moron4hire 10y ago
      Some of us are not such shitty programmers that we can't build critical systems on our own. Crtitcal business use is absolutely the worst time to farm out to a third party. 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."
      • zaphar 10y ago
        You seem to be offended when no offense was intended. No where did the OP indicate that this rule applied to Critical business use. JSON parsing is almost never a critical business use case. 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.
• deleted 10y ago
  [deleted]
• RangerScience 10y ago
  I had a need to write my own JSON parser for C#, although that was mostly because I hated the data structures the existing C# parsers produced. 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 :/
  • mort96 10y ago
    Wouldn't it have been easier to use the C# JSON parser, and then later walk the tree it creates and convert it into a more sane data structure that way?
    • RangerScience 10y ago
      Hmm. Yeah, it definitely would have been. All I can say is - I was frustrated with the way someone functioned, and rather than patch it, I "fixed" it. Where "fixed" is less complete and stable and... 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.
• legulere 10y ago
  There's nothing wrong with writing a JSON parser, but you shouldn't expect to do anything else for the next time if you want to do it correctly.
• Kiro 10y ago
  Why would you set the session before the password has been evaluated?
  • jjnoakes 10y ago
    Why do bugs exist?
• MaulingMonkey 10y ago
  > Well, first and most obviously, if you are thinking of rolling your own JSON parser, stop and seek medical attention. 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.)
• TeeWEE 10y ago
  Takeaway: Dont parse json. Every json parser is wrong. Json is not a standard. 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.
  • rhaps0dy 10y ago
    JSON does have a BNF definition. http://www.json.org/ http://www.json.org/
• rurban 10y ago
  Oh my, this is pure nonsense. 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-CONSIDERATIONS https://metacpan.org/pod/Cpanel::JSON::XS#SECURITY-CONSIDERA...