Invalid inputs causes authentication bypass in polyglot JSON messaging system?

Question

This is a very simple system based on sending JSON messages that seems to have a security vulnerability. There is a Python server (using JSON module included in standard library) which receives JSON objects and acts on them. If it gets {"req": "ping"}, it simply returns {"resp": "pong"}. It also has a command for setting volume, and one to change the admin password. The admin can send any JSON to this server. Here it is (server.py):

import json
import sys

def change_admin_password(p): pass # empty for test
def set_volume(v): pass # empty for test

def handle(js):
    if (js["req"] == "ping"):
        return {"resp": "pong"}
    if (js["req"] == "change admin password"):
        change_admin_password(js["args"]["password"])
        return {"resp": "ok"}
    if (js["req"] == "set volume"):
        set_volume(int(js["args"]["volume"]))
        return {"resp": "ok"}

print handle(json.load(sys.stdin))

It reads a command from stdin and processes it. Another script would read JSON objects from a network socket and pass them to this script.

Other users have to go through a proxy written in C++ using libjson. It simply blocks requests that should require admin privileges. For example, if a user tries to change the admin password, the proxy will reject the command:

$ echo '{"req": "change admin password", "args": {"password":"new"}}' | ./proxy 
echo $?
1

Here is the code (proxy.cpp):

#include "libjson.h"
using namespace std;
int main() {
    string json((istreambuf_iterator<char>(cin)), istreambuf_iterator<char>());
    JSONNode n = libjson::parse(json);
    string req = n.at("req").as_string();
    if (req == "change admin password") {
        return 1;
    }
    cout << n.write();
}

To use the proxy, the main script managing a socket would pipe data through the proxy and output of that to the Python server:

$ echo '{"req": "ping"}' | ./proxy | python server.py
{'resp': 'pong'}
$ echo '{"req": "set volume", "args": {"volume": 50}}' | ./proxy | python server.py 
{'resp': 'ok'}

And if a restricted command is attempted by the user, it will fail as expected:

$ echo '{"req": "change admin password", "args": {"password": "new"}}' | ./proxy | python server.py 
Traceback (most recent call last):
  File "server.py", line 17, in <module>
[...]

But for some reason, if the "req" key is in the JSON twice (shouldn't it be illegal?), non admin users are able to change the admin password:

$ echo '{"req": "nothing", "req": "change admin password", "args": {"password": "new"}}' | ./proxy | python server.py 
{'resp': 'ok'}

Why?

---

Trying Mike McMahon's workaround:

I tried using JSONNode.find as a workaround, but it doesn't seem to be working.

I tried just iterating over all elements:

#include "libjson.h"
using namespace std;
int main() {
    string json((istreambuf_iterator<char>(cin)), istreambuf_iterator<char>());
    JSONNode n = libjson::parse(json);
    for (JSONNode::json_iterator it = n.begin(); it != n.end(); it++) {
        cout << "found one: " << it->at("x").as_string() << endl;
    }
}

This works:

$ g++ proxy.cpp libjson.a  -o proxy && ./proxy
In file included from libjson.h:4:0,
                 from proxy.cpp:1:
_internal/Source/JSONDefs.h:157:6: warning: #warning , Release build of libjson, but NDEBUG is not on [-Wcpp]
{"a": {"x": 1}, "b": {"x": 2}}
found one: 1
found one: 2

Except it segfaults if the JSON is invalid? Am I using the iterator wrong?

$ echo '{"a": {"x": 1}, {"b": {"x": 2}}' | ./proxy 
found one: 1
Segmentation fault

I replaced the n.begin() with n.find("y"):

#include "libjson.h"
using namespace std;
int main() {
    string json((istreambuf_iterator<char>(cin)), istreambuf_iterator<char>());
    JSONNode n = libjson::parse(json);
    for (JSONNode::json_iterator it = n.find("y"); it != n.end(); it++) {
        cout << "found one: " << it->at("x").as_string() << endl;
    }
}

It doesn't work at all. Am I using the iterator wrong?

g++ proxy.cpp libjson.a  -o proxy && ./proxy
In file included from libjson.h:4:0,
                 from proxy.cpp:1:
_internal/Source/JSONDefs.h:157:6: warning: #warning , Release build of libjson, but NDEBUG is not on [-Wcpp]
{"y": {"x": 1}}
Segmentation fault

---

Another attempt at the workaround:

#include "libjson.h"
using namespace std;
int main() {
    string json((istreambuf_iterator<char>(cin)), istreambuf_iterator<char>());
    JSONNode n = libjson::parse(json);
    for (JSONNode::json_iterator it = n.begin(); it != n.end(); it++) {
        if (it->name() == "req" && it->as_string() == "change admin password")
        {
            cout << "found one " << endl;
        }
    }
}

It works!

$ echo '{"req": "change admin password"}' | ./proxy
found one 
$ echo '{"req": "x", "req": "change admin password"}' | ./proxy
found one 
$ echo '{"req": "change admin password", "req": "x"}' | ./proxy
found one 

But still segfaults with invalid JSON input?

$ echo '{"req": "x", {"req": "x"}' | ./proxy
Segmentation fault

Answer 1

known bug in libjson http://sourceforge.net/p/libjson/bugs/47/

---

Consider the JSON:

{ "same" : 4, "same": 5}  

• See http://www.jslint.com/

if you punch in the JSON, you will see an error (duplicate key names)

• Also see http://jsonlint.com/

if you punch in the JSON, you will NOT see an error, but the JSON will be reformatted and will remove the first "same" key.

A brief look-around indicates that the standard says you "should not" have duplicate key names, not "must not", so technically libjson is ok.

Your API (ie at(string), operator) is not consistent with either of those two websites (above).

• jslint shows an error
• jsonlint kills off the first node, leaving the node with the value 5, which I suppose is consistent with javascript's overriding behaviour.

the libjson.at(string) function will return the FIRST entry (rather than the last).

Workaround with JSONNode.find("req")

Look at the find method

JSONNode.find("req"); 

Returns a pointer to a array of nodes you can use to determine if it has been specified more than once. While the library should overwrite each entry on top of the subsequent one (and is a bug to not do so, per valid JSON) - you can use the find method to locate and obtain an iterator to step over the matching nodes.

JSONNode n = libjson::parse(json)
JSONNode::json_iterator it = n.find("req");
// iterate over the array
if (it[i].at("req").as_string() == "change admin password") {
  return 1;
}

note this is not the best workaround in the world as i would imagine this returns all nodes matching the argument and in a complex structure there may be more than one node with the same name nested somewhere else, for your needs this should suffice.

However you should look into another library for validating your JSON calls if this is critical, or possibly support an admin distinguished command, such as areq for an administrative request. This would allow you to scrub out any requests at the proxy that contain the areq command as only administrators would be able to generate such request (and obviously not send through the proxy).

Any standard requests containing administrative commands would just fail off.

Update

Try using the iterator as so, admittedly C++ is not my primary language and i'm not well versed with STL iterators.

for (JSONNode::json_iterator jsonIter = n.begin(); jsonIter != n.end(); jsonIter++) {
    if (jsonIter->name() == "req" &&
        jsonIter->as_string() == "change admin password") 
    {
        // found something do magic here
    }
}

The aforementioned code worked for me in my testing.

---

illegal or not you need to scrub for this in your proxy and in your server. it is abusing a likely vulnerability in most libraries that concatenate the keys down (as you cannot have duplicate keys). Look for it and scrub it out anywhere it's not being treated as a JSON object.

OR put another layer before your proxy that takes the JSON and runs it through a parser - which would remove any duplicate keys.

EDIT

JSON lib in python is getting the first defn of req and applying it's content, then upon the second definition of req it is overwriting the first and setting the exploiters commands.

Since your python app blindly assumes that any incoming request has been pre-scrubbed (bad idea) you'll need to scrub for this at the proxy layer, and possibly look into using an API key for admin level requests, so that anything that gets through or bypasses your proxy would still need to know a KEY of some sort.