Javascript json eval() injection

Question

I am making an AJAX chat room with the guidance of an AJAX book teaching me to use JSON and eval() function. This chat room has normal chat function and a whiteboard feature. When a normal text message comes from the php server in JSON format, the javascript in browser does this:

Without Whiteboard Command -------------------------------------------

function importServerNewMessagesSince(msgid) {
    //loadText() is going to return me a JSON object from the server
    //it is an array of {id, author, message}
    var latest = loadText("get_messages_since.php?message=" + msgid);
    var msgs = eval(latest);
    for (var i = 0; i < msgs.length; i++) {
                    var msg = msgs[i];
                    displayMessage(escape(msg.id), escape(msg.author), escape(msg.contents));
    }   ...

---

The whiteboard drawing commands are sent by server in JSON format with special user name called "SVR_CMD", now the javascript is changed slightly:

With Whiteboard Command --------------------------------------------------

function importServerNewMessagesSince(msgid) {
    //loadText() is going to return me a JSON object from the server
    //it is an array of {id, author, message}
    var latest = loadText("get_messages_since.php?message=" + msgid);
    var msgs = eval(latest);
    for (var i = 0; i < msgs.length; i++) {
                    var msg = msgs[i];
                    if (msg.author == "SVR_CMD") {

                        eval(msg.contents);  // <-- Problem here ...

                         //I have a javascript drawLine() function to handle the whiteboard drawing
                        //server command sends JSON function call like this: 
                        //"drawLine(200,345,222,333)" eval() is going to parse execute it
                        //It is a hacker invitation to use eval() as someone in chat room can
                        //insert a piece of javascript code and send it using the name SVR_CMD?

                   else {
                        displayMessage(escape(msg.id), escape(msg.author), escape(msg.contents));
                    }

    }   ...

---

Now, if the hacker changes his username to SVR_CMD in the script, then in the message input start typing javascript code, insdead of drawLine(200,345,222,333), he is injecting redirectToMyVirusSite(). eval() will just run it for him in everyone's browser in the chat room. So, as you can see, to let the eval to execute a command from an other client in the chat room is obviously a hacker invitation. I understand the book I followed is only meant to be an introduction to the functions. How do we do it properly with JSON in a real situation?

e.g. is there a server side php or .net function to javascriptencode/escape to make sure no hacker can send a valid piece of javascript code to other client's browser to be eval() ? Or is it safe to use JSON eval() at all, it seems to be a powerful but evil function?

Thank you, Tom

Answer 1

What is this book? eval is evil, there is not a single reason to use it, ever.

To transform a JSON string into a javascript object, you can do the following:

var obj = JSON.parse(latest)

Which means you can then use:

[].forEach.call(obj, function( o ) {
    // You can use o.message, o.author, etc.
} )

To do the opposite (javascript object -> JSON string), the following works:

var json = JSON.stringify(obj)

Answer 2

It only is unsafe if the executed code is generated by other clients and not by the server. Of course you would need to prevent anybody to use that name, though I don't understand why you would use the "author" field? Just send an object {"whiteboard":"drawLine(x,y,z)"} instead of {"author":"SVR_CMD","contents":"drawLine(x,y,z)"}.

But it is right, eval() is still an invitation for hackers. One can always send invalid data and try to influence the output more or less directly. The only way for escaping is a proper serialisation of the data you want to receive and send - the drawings data. How do you receive the whiteboard commands? There is no serverside "escape" function to make javascript code "clean" - it would always be a security hole.

I would expect a serialisation like

message = {
    "author": "...", // carry the information /who/ draws
    "whiteboard": {
         "drawline": [200, 345, 222, 333]
    }
}

so you can sanitize the commands (here: "drawline") easiliy.

The use of eval() might be OK if you have very complex commands and want to reduce the transferred data by building them serverside. Still, you need to parse and escape the received commands from other clients properly. But I'd recommend to find a solution without eval.