5

Have a look at this simple JSON system composed of a user facing service that talks to a back end:

import java.util.*;
import com.json.parsers.*;
import org.json.simple.*;

class Main{static {
    String s = new java.util.Scanner(System.in).useDelimiter("\\A").next();
    UserFacingService.handleMessage(s);
}}

class UserFacingService {
    static void handleMessage(String s) {
        JSONParser parser = JsonParserFactory.getInstance().newJsonParser();
        Map js = parser.parseJson(s);
        if (commandAllowed(js))
            Backend.handleMessage(s);
        else
            System.out.println("unauthorized!");
    }
    static boolean commandAllowed(Map js) {
        if (js.get("cmd").equals("bad stuff"))
            return false;
        return true;
    }
}

class Backend {
    static void handleMessage(String s) {
        JSONObject js = (JSONObject) JSONValue.parse(s);
        String cmd = (String)js.get("cmd");
        if (cmd.equals("bad stuff")) {
            doBadStuff();
        } else if (cmd.equals("ping")) {
            doPong();
        }
    }
    static void doBadStuff() { System.out.println("doing bad stuff"); }
    static void doPong() { System.out.println("doing pong"); }
}

The user facing service (using quick-json) prevents the user from making the backend (using json-simple) do bad stuff. To simulate the user, I'm using stdin from Main. In a real scenario, Backend would be on another server, or some old legacy / binary code, so we can't simply change it to accept Java objects. The design is that authorization is done separately from Backend. This could be elaborated by making another class called AdminFacingService that only the admin has access to, or by putting if (userIsAdmin) return true; at the top of UserFacingService.commandAllowed.

As you can see, it works as intended:

$ CP=.:json-simple-1.1.1.jar:quick-json-1.0.2.3.jar
$ javac -cp $CP A.java && echo '{"cmd": "ping"}' | java -cp $CP Main
doing pong
Exception in thread "main" java.lang.NoSuchMethodError: main

Sending {"cmd": "bad stuff"} should cause the user facing service to reject the command:

$ CP=.:json-simple-1.1.1.jar:quick-json-1.0.2.3.jar
$ javac -cp $CP A.java && echo '{"cmd": "bad stuff"}' | java -cp $CP Main
unauthorized!
Exception in thread "main" java.lang.NoSuchMethodError: main

That works as well. But then if the user sends {"cmd": "bad\u0020stuff"}, bad stuff happens:

$ CP=.:json-simple-1.1.1.jar:quick-json-1.0.2.3.jar
$ javac -cp $CP A.java && echo '{"cmd": "bad\u0020stuff"}' | java -cp $CP Main
doing bad stuff
Exception in thread "main" java.lang.NoSuchMethodError: main

But how? Why isn't the user facing service catching this?

Dog
  • 7,707
  • 8
  • 40
  • 74

4 Answers4

2

Right, I see. You're using two different parsers. Originally I though these were lenient in different ways. "Postel's Law" is great for UIs but dangerous for programming interfaces. In particular, hairy parsing is great for creating vulnerabilities.

Looking (again!) at RFC4627, \u notation is legal in JSON. One of your parsers appears to be broken and interprets the data incorrectly.

Generally only parse once - quite a few bugs come from multiple parsing. If you need the data in JSON again, extract and re-encode using a formatter that is clearly within spec and any buggy implementations you have to deal with.

It's kind of interesting, that even though is supposed to be really simple, text formats are susceptible to this sort of thing. So, prefer simple (not ASN.1) binary formats and use quality libraries.


Original answer, in which I wrongly assumed you were starting a new process every time with user data on the command line:

This is nothing to do with parsing JSON and everything to do with what you put on the command line. It's a bad idea to put untrusted data on the command line. This is in the "injection" class of vulnerabilities, along with SQL injection, XSS, LDAP injection, HTTP request/response splitting and many others. Most of these vulnerabilities can be solved by a tight formatting library. Command lines are all over the place, so it's best to either avoid them completely or go for something like Base64 encoding.

OWASP have a page on Command Injection.

The "Secure Coding Guidelines for the Java Programming Language, Version 4.0" has Guideline 3-4: Avoid any untrusted data on the command line.

(You're also very easily leaving yourself open to a DoS attack if you start a new Java process for every request.)

Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305
  • `Main` was just for testing. The idea is that in real code, commands would come from clients on the network, and get passed to `UserFacingService.handleMessage` (There wouldn't be any spawning of processes for each command). If you change `s` in main to `"{\"cmd\": \"bad\\u0020stuff\"}"`, the bug still happens. – Dog Oct 31 '13 at 00:35
  • 1
    Right, I see. You're using two different parsers, which are lenient in different ways. Looking at RFC4627, `\u` notation is illegal in JSON, and your parsers are being lenient in interpreting in different ways. As ever, "Postel's Law" is great for UIs but dangerous for programming interfaces. Generally only parse once - quite a few bugs come from multiple parsing. If you need the data in JSON again, extract and re-encode using a formatter that is clearly within spec (and buggy implementations), – Tom Hawtin - tackline Nov 15 '13 at 15:09
  • @TomHawtin-tackline I'm confused - doesn't RFC4627 section 2.5 cover `\u` notation? Why do you say it's illegal? – artbristol Nov 18 '13 at 14:44
  • @artbristol Oh, so it does. Probably searching for `u instead of \u - Mac keyboard next to a PC. – Tom Hawtin - tackline Nov 18 '13 at 14:51
  • Everything I can think of follows Postel's law. How specifically does it cause a vulnerability in this case? – Dog Nov 18 '13 at 16:46
2

With bad\u0020stuff as an input, the first parser returns an object js where js.get("cmd") returns the string bad\u0020stuff, so commandAllowed returns true. The second parser returns an object js where js.get("cmd") returns bad stuff (it converts \u0020 to a space). Thus the bad stuff command is executed.

The first parser is not following RFC4627, because it's not interpreting unicode escapes.

Harold R. Eason
  • 563
  • 2
  • 9
0

It seems likely to me that it's a bug in quick-json.

quick-json doesn't escape \ properly:https://code.google.com/p/quick-json/issues/detail?id=6

Therefore I reckon that it doesn't unescape it properly either. So stick to json-simple which is more compliant with the spec.

artbristol
  • 32,010
  • 5
  • 70
  • 103
0

If you're unable to use a JSON library that properly follows the spec then ...

org.apache.commons3.lang.StringEscapeUtils.unescapeJava(cmd).equals("bad stuff")

Check this SO question How to unescape a Java string literal in Java?

Community
  • 1
  • 1
Louis Ricci
  • 20,804
  • 5
  • 48
  • 62