Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10333 closed enhancement (fixed)

fromJson should not eva() without check

Reported by: Ben Blank Owned by: anonymous
Priority: high Milestone: 1.6
Component: Core Version: 1.3.2
Keywords: Cc: kriszyp, elatzukin
Blocked By: Blocking:

Description (last modified by dante)

Currently, the fromJson method blindly eval()s any string passed to it, a potentially dangerous operation (depending on the string's source, it could easily contain injected code). A simple RegExp check can be used to prevent this: any string which does not match

`/^(?:[,:{}\[\]0-9.\-+Eaeflnr-u \n\r\t]|"(?:\\.|[^"\\])*")*$/` 

is not valid JSON and should not be eval()ed. (Note that the reverse is not true; that expression can match invalid JSON, but it at least won't be malicious — no function calls, control structures, etc.)

Adding the line

`if (!/^(?:[,:{}\[\]0-9.\-+Eaeflnr-u \n\r\t]|"(?:\\.|[^"\\])*")*$/.test(json)) throw "invalid JSON";` // (or some such) 

to fromJson would prevent this kind of script injection. It is also a cheap operation: it only traverses the string once and fails early if an invalid character is found.

Change History (4)

comment:1 Changed 10 years ago by dante

Cc: kriszyp elatzukin added
Description: modified (diff)
Milestone: tbdfuture

providing a secure JSON path is on the radar, but we can't do it in fromJson because of backward compatibility. There is already a secure parser in dojox.secure.

Perhaps:

dojo.fromJsonSecure = function(json){
    if(!/thatregexp/.test(json){
       return dojo.fromJson.apply(dojo, arguments);
    }
}

would be a small base addition worth providing?

comment:2 Changed 10 years ago by dante

Additionally, making XHR and friends json-safe would be a matter of making a new contentHandler:

d.contentHandlers["json-secure"] = function(xhr){
    return dojo.fromJsonSecure(xhr.responseText);
}

comment:3 Changed 9 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

(In [22926]) Added dojox.secure.fromJson for safe JSON parsing, fixes #10333, refs #8111

comment:4 Changed 9 years ago by bill

Milestone: future1.6
Note: See TracTickets for help on using tickets.