My JavaScript book is out! Don't miss the opportunity to upgrade your beginner or average dev skills.

Tuesday, April 03, 2007

Are 130 byte enought to solve JavaScript JSON Hijacking problems? (Unlikely not this time)

This is the second time I open this post because I didn't do a good debug but my solution, a sort of personal brainstorming, was good enough to open one more time this post after few changes :)

This is my proposal:

(function(m){function $(c,t){t=c[m];delete c[m];try{eval(""+c)}catch(e){c[m]=t;return 1}};return $(Array)&&$(Object)})("toString")

exactly 130 byte to solve (I suppose) Array and Object modified constructor problems.



(function(m){function $(c,t){t=c[m];delete c[m];try{new(function(){}).constructor("",""+c)}catch(e){c[m]=t;return 1}};return $(Array)&&$(Object)})("toString")

exactly 158 byte to solve (I suppose) Array and Object modified constructor problems.

Let me explain this function concept.

Step 1 - There's no way to know if a constructor is native
This is the first problem, we can't believe on a generic object constructor for, at least, these two reason:

  1. a constructor, as I wrote on MDC too, is not read only

  2. there isn't any official method to know if a part of code is native or not, and if it is implemented, it should be changed by malicious arbitrary code


Since a constructor should be redefined in some browser (first of all, FireFox)
function Array(){ /*doStuff*/ };

is really hard to know if this new constructor is native code or not.
You could create a new original, empty, scope (using for example an iframe), then You can check if a variable constructor, in this new scope, is equal to original one, for example, a generic Array in a generic window.
The only way to know if a constructor was inherited from JavaScript core is its string rappresentation, usually something like:

function Array() {
[native code]
}

This is a common native code string rappresentation, but as You know JavaScript is object oriented and every object has a native toString method that should be override/overwite as every other public method.

function Array(){};
Array.toString = function(){
return "function Array(){\n\t[native code]\n}"
};

alert(Array);

This means that we can't believe on string rappresentation too ... and that's why I thought about next step.



Step 2 - native code cannot be evaluated
The uniq way to know if a method or a constructor is inherited from JavaScript core is the "[native code]" string inside its string value.
At the same time You can't evaluate a native code because it isn't a replicable JavaScript code, so a try...catch like this one should be a trick to know if code is native or not because in this last case it will not make code executable:

function isNativeCode(methodOrConstructor){
var result = false;
try{eval(""+methodOrConstructor)}
catch(e){result = true};
return result;
};

alert([
isNativeCode(Array), // true
isNativeCode(String.replace), // true
isNativeCode((1).constructor), // true
isNativeCode(function(){}) // false
]);

As I said, overwriting toString method should produce a sort of native code simply and quickly:

function Mine(){};
Mine.toString = function(){
return "[native code]"
};

alert(isNativeCode(Mine)); // true

This is the reason I thought about next final step.


Step 3 - enumerable methods should be deleted!
At this point, the biggest problem is the ability to reply native code string, using for example a toString method.
Is it possible to solve? Likely sure, using a delete keyword!

function Array(){};
Array.toString = function(){
return "I'm an Array";
};

alert(Array); // I'm an Array

delete Array.toString;

alert(Array); // function Array(){\n}

Amazing, I can remove arbitrary toString method to know original value ... but does delete work without problems with native constructor too? Of course, it's on core ;)

alert(Array.toString());
// function Array(){\n\t[native code]\n}

delete Array.toString;

alert(Array.toString());
// function Array(){\n\t[native code]\n}

Well done ... but this should be quite obtrusive because some library (or some developer) should redefine toString method for its scope ... that's why I decieded to assign one more time toString method, after deleting them, obviously ... and this is my first proposal, with details:

// anonymous private scope function, accept one argument
(function(m){

// private useful dollar function
// accepts 2 arguments, one for constructor
// and one for lazy programming
function $(c,t){

// second argument is used to save old toString method
t=c[m];

// delete removes constructor toString
delete c[m];

// check if constructor contains native code
// (not usable, not evaluable)
try{eval(""+c)}

// if eval fails, constructor contains
// [native code]
catch(e){

// so I can add safely old toString method
// (is not a problem if it is native or not)
c[m]=t;

// and return a "true" value
return 1
}
};

// at this point I just need to call
// my dollar function to know
// if Array and Object are native constructors
return $(Array)&&$(Object)
})
// for size reasons, I send toString method name
// using "m" var multiple times inside function
("toString")


How to use this runtime private scope function?
This is just an example that should be used before every JSONString to value convertion:

if((function(m){function $(c,t){t=c[m];delete c[m];try{eval(""+c)}catch(e){c[m]=t;return 1}};return $(Array)&&$(Object)})("toString"))
alert("I can decode a JSON string");
else
alert("JSON decoding is corrupted");


And that's all folks, You can view a basic example in this page and a failed hack example in this one.

Finally, this inline function should work with every Ajax ready browser, successfully tested on IE 5, 5.5, 6, 7, FireFox 1, 1.5, 2 and Opera 8, 9.

I'm waiting for Safari 1 or 2 behaviour, as konqueror or other browsers (but I'm quite sure, this time this proposal should work correctly).

If You find a way to crack this solution, please tell me (us) :D


Update - 04/04/2007
This is a revisited version dedicated for XMLHttpRequest native object

if((function(c,m,t){t=c[m];delete c[m];if(/^\[XMLHttpRequest\]$/.test(c)){c[m]=t;return 1}})(XMLHttpRequest,"toString"))
alert("Valid XMLHttpRequest");
else
alert("XMLHttpRequest is corrupted");


Same concept, same logic ... and should be as secure as first proposal for Array and Object constructors.

Try them:

function XMLHttpRequest(){};
XMLHttpRequest.toString = function(){
return "[XMLHttpRequest]"
};
XMLHttpRequest.constructor = Object;

if((function(c,m,t){t=c[m];delete c[m];if(/^\[XMLHttpRequest\]$/.test(c)){c[m]=t;return 1}})(XMLHttpRequest,"toString"))
alert("Valid XMLHttpRequest");
else
alert("XMLHttpRequest is corrupted");



Update - 04/04/2007
Do You worry about eval?

if(eval === (new function(){}).eval)
alert("OK eval");
else
alert("eval is corrupted");

And now we just need to pray that deprecated object.eval will survive for some year :D



Instant Update - 04/04/2007
damn ... Object.prototype.eval should redefine generic object eval method ... uhm, please let me think about a solution and sorry for precedent update.


Update - 05/04/2007
kentaromiura gives me a nice idea ... He wrote them next post.
Function can eval() code in a safe way.
You can redefine Function itself but in this case native behaviour will be lost so I suppose this should be the better way to be sure that code evaluation and Array/Object constructors are not modified.

var result, code = '[1,2,3]';
if((function(m){function $(c,t){t=c[m];delete c[m];try{new Function("",c)}catch(e){c[m]=t;return 1}};return $(Array)&&$(Object)})("toString"))
result = new Function("","return "+code)();

Is this the securest way, for JavaScript, to know if code has been cracked?


Instant Update
no man ...
function Function(a,b){
// Array = function(){} ...
eval("function f(){"+b+"}");
return f;
};

alert(new Function("","return [1,2,3]")());

:(


Important Update - 05/04/2007
I found a way to solve eval problems, look here to know more.
This is the last version, it should be the best way to know if Array and / or Object were cracked.

/*
function Array(){};
Array.toString = function(){
return "function Array(){\n\t[native code]\n}"
};
//*/

if(
(function(m){function $(c,t){t=c[m];delete c[m];try{new(function(){}).constructor("",""+c)}catch(e){c[m]=t;return 1}};return $(Array)&&$(Object)})("toString")
)
alert("Array and Object constructors are OK");
else
alert("Corrupted Array and Object constructor");


With this concept I should be shure that XMLHttpRequest object is native and not re-defined.

Here You can read my last proposal for XMLHttpRequest check:

if((function(x,c,m,t){t=x[m];delete x[m];if(new(function(){})[c]("c","return ("+x+")[c]!==(function(){})[c]")(c)){x[m]=t;return 1}})(XMLHttpRequest,"constructor","toString"))
alert("XMLHttpRequest is OK");
else
alert("XMLHttpRequest is Corrupted");


It seems to be safe as function(){} code evaluation is.
If XMLHttpRequest is not native, it's a function and removing, using delete, its toString method, it should be a constructor and a constructor has a function(){} constructor, isn't right?

21 comments:

digitarald said...

I tried a similar check for XMLHttpRequest, deleting and and having a fresh and clean XMLHttpRequest. It does not work in Firefox Mac. I'm anxious to see results for Firefox Mac.

Andrea Giammarchi said...

XMLHttpRequest? Even shorter ;)

if((function(c,m,t){t=c[m];delete c[m];if(/^\[XMLHttpRequest\]$/.test(c)){c[m]=t;return 1}})(XMLHttpRequest,"toString"))
alert("Valid XMLHttpRequest");
else
alert("XMLHttpRequest is corrupted");

There's only a limit, You can't reproduce XMLHttpRequest for old browser.

tobie said...

Hi,

If I'm not mistaking, this completely fails to handle the security issue at hand which relies on grabbing the content via the dynamic script tag. See this post for more information.

Andrea Giammarchi said...

As I said many times on different sites, if someone can inject JavaScript the attack is just "successful completed", with or without Ajax.

If the problem is, in this case, JSON evaluation because someone could change native Array or Object constructor, with my proposal You'll be sure these constructors were not changed.

At the same time, if the problem is a modified XMLHtpRequest, with last update You'll be sure it's original native version.

These function are obviously more secure as server-side response, to prevent code changes injected by malicious cracker.

With this concept You can choose to send to a JavaScript callback a JSON value, verifying that this callback isn't changed, verifying that XMLHttpRequest is native and both Array and Object constructor weren't modified.

Finally, this is a proposal, not a perfect solution because if "You" don't know how to make your pages more secure, there's no reason to care about Ajax because probably "your" page is not secure even with simple GET requestes :)

tobie said...

Andrea, we're not talking about a regular XSS attack here, hop to Andrew's comment to get what I mean and why your solution doesn't solve this issue at all, while the commenting your JSON responses does.

Andrea Giammarchi said...

Since Carol can't read the JSON response in as a string, she has to hook into the JS interpreter herself by overriding a constructor for a native datatype (like Array or Object).

My proposal is oriented to server response and in this way Carol can't change Array or Object constructor.

However, about the problem, I never used cookie to authenticate a user because cookie are not safe from different years.

I agree this is a strange and big problem but I suppose there are different ways to solve them, expecially on server side, not on client :)

Anonymous said...

i haxored it, later, kenta

Andrea Giammarchi said...

i haxored it, later, kenta
... ehr ... what? (You haxored my bad English too)

Anonymous said...

Interesting.

But what about someone redefining the eval() function itself? Should be easy, maybe by adding some logic to recognise own compromised code and in that case to throw an exception confusing your test?
(Ok, there are obstacles for such an attack, because for some reasons you can´t delete a modified eval-function the way you proposed for the toString methods. I quickly checked: In FF2 it stays the modified function, in IE6 it is completely deleted as for window.eval in Opera9.1. Just deleting and calling eval() crashes Opera. Sigh... But anyways, as a result at least FF is resistant to your method of deleting methods to find out if they are native.)

Another thing. If you rely on unchanged XHR, Array and Object, why not simply delete them (maybe with backup) just before any JSON eval and deal with the native objects? If this is not possible as for XHR for FF Mac, as digitarald said, maybe you could hijack a guaranteed native object out of a subframe as Dean Edwards described in http://dean.edwards.name/weblog/2006/11/hooray/ (also look at the comments)

Andrea Giammarchi said...

nice point above ...

FireFox resists correctly while the reason I didn't simply delete original object is that:

1 - IE, after delete Array, works correctly with [] but stop working with var ar = new Array;

2 - if You delete a native function You'll remove every prototype ... and it should be a bit obtrusive, don't You agree?

It seems that eval attack is my real solution limit ... now I'm thinking how to solve and probably an iframe should resolve the problem, stay tuned :D

Andrea Giammarchi said...

HTMLBodyElement.prototype.appendChild = function(){};

With FireFox You can delete everything but You can define HTML elements methods too so Dean proposal seems to be not more safe than mine :(

tobie said...

My proposal is oriented to server response and in this way Carol can't change Array or Object constructor.

Carol's on her own page. She's using a script tag to load the pirated data there so eval is not used. The pirated data is automatically evaluated by the browser (since it's called from a script tag).

Your post title does read: "Are 130 byte enought to solve JavaScript JSON Hijacking problems?". The answer to this question is clearly no; as you state yourself, some server side considerations are also to be taken.

I would watch out not to mislead people on security issues, that's part of your responsability as a blogger.

Andrea Giammarchi said...

The pirated data is automatically evaluated by the browser (since it's called from a script tag).
exactly, so an if like that will block pirate if He changed Array or Object constructor, isn't right?

However ...
I would watch out not to mislead people on security issues, that's part of your responsability as a blogger.
of course, but this post title ... is a question, with a proposal, not a repsonse.
I wrote them to talk about the problem and try to find a solution, client or server.

Andrea Giammarchi said...

Last personal opinion: I think usage of POST instead of GET isn't a good solution and my proposal is not a good solution too.

Usage of session requires tests because if You don't close your browser session_id will be the same, isn't right?

If You encrypt data malicious code should find the key, isn't right?

Well, just talk about these problems :)

kentaromiura said...

see my eval proposal in the other post..

for the post vs get
if you use post you are absolutely not safe.

it have the same problems of get.

Anonymous said...

hope someone comes out with a solution simpler than discarding json .

Anonymous said...

andrew if you redefine even eval:
eval=function(code){
return new Function("","return "+code)();
}
then you got no problem, because
if someone redefine the Function
the program go in a infinite loop
^_^; kenta;

btw, i think that solving using eval is no good, cause you're taking for sure that XMLHTTPRequest is a good one..

if someone could redefine XMLHTTPRequest then the problem that you're trying to solve here cause ever worse problem...

Andrea Giammarchi said...

I solved code evaluation problems ... now I should solve the rest :D

Julien Couvreur said...

I believe that the delete trick can be fooled:

var obj1 = { "a" : 1, "b" : 2 };
function Object2() { this.a = 3; }
Object2.prototype = obj1;
var obj2 = new Object2();

obj2.a // returns 3
delete obj2.a
obj2.a // returns 1

Andrea Giammarchi said...

I believe that the delete trick can be fooled

did You try Object, Array or Function toString method too, defining them as a prototype of another object ?

It's should be interesting but It shouldn't work with global Object too (i.e. delete eval; alert(eval);)

Dan said...

It is pointless to try to mitigate this problem on the client-side. JavaScript was not made to hide things from the page. The only thing the client can be made to do is provide the server with some piece of information to prove it is legitimately allowed access by the user. (Such as sending the session_id through the JSON request. The JavaScript can't know that information unless the server has given it to the domain that the JavaScript is running on.)

Specifically, checking to see if objects have been changed is like "trying to find a needle in a hay-stack". There are too many ways to change them. Not the least of which is to replace Object.prototype with some new object. (Note: this will not create an infinite loop because the new object would be created before the assignment.)

Finally, replacing Object.prototype might be a desirable feature in the client code. In such a case, the programmer is restricted too much.