I agree with you, "while(1);" is better. :-) A bit uglier, but you are
correct that some data could get out of the comment.
The problem is that the evalJSONRequest() don't use the evalJSON()
function to evaluate the JSON. This is bad, because there are in fact
two ways of evaluating JSON on using evalJSON() and other with
evalJSONRequest(). I propose to aways use the evalJSON function to
evaluate JSON strings.
The following diff implements what I suggested :
Index: Base.js
===================================================================
--- Base.js (revision 1278)
+++ Base.js (working copy)
@@ -793,7 +793,8 @@
/** @id MochiKit.Base.evalJSON */
evalJSON: function () {
- return eval("(" + arguments[0] + ")");
+ var json = arguments[0].replace(/^while\(1\);/, "");
+ return eval("(" + json + ")");
},
/** @id MochiKit.Base.serializeJSON */
Index: Async.js
===================================================================
--- Async.js (revision 1278)
+++ Async.js (working copy)
@@ -217,7 +217,7 @@
MochiKit.Base.update(MochiKit.Async, {
/** @id MochiKit.Async.evalJSONRequest */
evalJSONRequest: function (/* req */) {
- return eval('(' + arguments[0].responseText + ')');
+ return evalJSON(arguments[0].responseText);
},
/** @id MochiKit.Async.succeed */
===================================================================
On Apr 3, 10:57 am, "troels knak-nielsen" <[EMAIL PROTECTED]> wrote:
> It can be done almost transparently, by simply replacing
> /^while\(1\);/ with "" in evalJSON(). This would leave data without
> the safeguard as-is. The only code it would brake, would be code that
> relied on having such code running, and that would be insane anyway
> ...
>
> I prefer while(1); over commenting code for two reasons. a) It
> generates an error (script hangs) on the malicious site. b) if a
> string contains the characters */, it would escape out of the
> safeguard with the commenting solution.
>
> On 4/3/07, Victor Bogado <[EMAIL PROTECTED]> wrote:
>
>
>
>
>
> > This has worried me also.
>
> > The problem is that the json load could be included in a rogue web
> > page by using a <script> tag. Since the author of this page has total
> > control of what he will display he can subvert the javascript
> > interpreter to run whatever he like when the json object is executed
> > (changing the array or object constructor).
>
> > There are two solutions to this problem, one requires that the client
> > side to prove that he actually knows the cookie by embedding some part
> > of it in the URL of the request and denying people who don't know.
> > Since Mochikit is not concern with the server side this does not
> > really apply here.
>
> > The second solution implies that JSON should not be directly runnable,
> > by prefixing it with a 'while(1)' or commenting the whole thing for
> > instance would be enough. But motchikit makes this hard by providing a
> > simple, very simple, to use "loadJSONDoc()" function.
>
> > I am a person who cares a lot with security, and if I were in charge
> > of motchikit I would change the definition of the default JSONDoc used
> > by kit to be a commented out javascript and would change the
> > evalJSONRequest function to uncomment the data before it evaluate it.
> > The problem is that this would break current behavior, in my opinion
> > this is not bad, because it is simple enough to implement this on
> > whatever server-side application you have and would also force the
> > applications to use a more secure form of JSON.
>
> > Another way is to call this new form of JSON something else like SJSON
> > (secure json?) or CJSON (Commented JSON, witch is more honest since
> > we can't know if this is 100% secure anyway) and create a
> > evalCJSONRequest and loadCJSONDoc. This way it would not break the
> > current insecure way of doing things but would give simple tools to
> > people that want security on their sites to operate.
>
> > My opinion is that the fisrt option is the best, I made a quick hack
> > that do just that (I don't have the time to test it right now). the
> > diff to svn version is posted bellow :
>
> > Index: Async.js
> > ===================================================================
> > --- Async.js (revision 1278)
> > +++ Async.js (working copy)
> > @@ -217,7 +217,9 @@
> > MochiKit.Base.update(MochiKit.Async, {
> > /** @id MochiKit.Async.evalJSONRequest */
> > evalJSONRequest: function (/* req */) {
> > - return eval('(' + arguments[0].responseText + ')');
> > + var re = new RegExp("^\s/\*(.*)\*/\s*$");
> > + var json = arguments[0].responseText.replace(re, "$1");
> > + return eval('(' + json + ')');
> > },
>
> > /** @id MochiKit.Async.succeed */
> > ===================================================================
>
> > On Apr 3, 6:35 am, "troels knak-nielsen" <[EMAIL PROTECTED]> wrote:
> > > This might be of interest.
>
> > >http://www.fortifysoftware.com/servlet/downloads/public/JavaScript_Hi...
>
> > > --
> > > troels
>
> --
> troels
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"MochiKit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
