#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 )
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 11 years ago by
Cc: | kriszyp elatzukin added |
---|---|
Description: | modified (diff) |
Milestone: | tbd → future |
comment:2 Changed 11 years ago by
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 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 Changed 10 years ago by
Milestone: | future → 1.6 |
---|
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:
would be a small base addition worth providing?