I posted this to the websec mailing list and I thought I'd post it here to see if anyone could beat it:-

Over the last few months I've been developing and rewriting (a lot) JSReg but now hopefully I'm finally getting somewhere. The goal was to produce a sandboxed version of Javascript within Javascript itself because I need a sandbox for some projects I'm working on and I don't want the overhead of another language.

My sandbox works with prefixes and suffixes so "x" becomes "$x$" and any reference to objects becomes $obj[$+'yourref'+$]. In addition I only allow certain functions/properties based on a whitelist (so stuff like constructor isn't supported). I also create safe functions which run some checks to prevent window leakage, for example take (1,[].sort)().alert(1) here we leak to window. I protect against this sort of attack by whitelisting native functions to disallow no or null arguments with the option to override per function (then an additional check is performed on the object).

JSReg contains a special object called "globals", I use this to rewrite your javascript code so for example 'test' becomes globals.string('test') this produces a special prototyped version of the string which can be used later. Native functions are also supported this way by calling their name e.g. globals.alert(1)

So how does the code look once it's been JSReg'd well here is a code sample:-
function x(){ var m=1; this.getM=function(){ return m; } }; y=new x; y.getM()

Which gets rewritten to:-
function $x$(){ var $m$=globals.number(1);this.$getM$=function(){ return $m$; } };$y$=new $x$;$y$.$getM$()

As you may have noticed I allow "this" to be used in this way but I will disallow assignment or return the value of "this", I may improve this in future once I'm certain that it is safe to use.
At the moment I allow JSReg globals to be overwritten but I might prevent this at the regexp level or with setters and there are a few problems when not finishing a statement with a ";". Finally there's a limitation regarding the scope, at the moment the prefixes and suffixes are in the global scope so $x$ is actually window.$x$, I plan to get round this somehow so that $x$ is assigned to a object I'm still working that out.

Any comments or suggestions are of course welcome but specifically I'm looking for hacks to window or glaring errors in my RegExps. If you can hack JSReg so that it returns window please let me know

Once I'm confident that it is a secure sandbox, I shall release it as open source. You can have a go here:-
http://www.businessinfo.co.uk/labs/jsreg/jsreg.html

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

(<>JSReg rox</>,[].concat)()[0]

:)

Nice work!

I guess there is a bit of work in identifying the [] notion. Although no cookies until I see [object Window] :)

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

I wonder what JS 1.7 and 1.8 will bring regarding window leakage. I haven't found time to play yet but I am sure let, yield, generator expressions etc will be fun.

Well Firefox is pretty bad atm for leaking window. It seems anywhere which allows a function in an argument for a native function it's possible to gain window. let could have scoping issues for me but I'm not sure I'll support it yet. Hopefully FF3.5 will fix these problems

Update...
Fixed the vector! Nice find again :) Still works I need to work on the regexps for arrays. Damn

Here are some vectors I found:-
x=[1,2,3]//;
['sort']

[[[[x=alert]]]]

x=[1,[[x=alert]],2]

These should be fixed although arrays is still a bit buggy

Update..
Ok Mario I think I've fixed it. This was really tough but really useful thanks. I've added comments removal too

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]



Edited 4 time(s). Last edit at 06/30/2009 03:34PM by Gareth Heyes.

Quote

([],(new function(){
this.z=function(){
// this==window
}
}).z)()

// to fix this you have to rewrite calls in the form: asdf(123) to globals.fn($asdf$,[globals.number(123)]) and new asdf(123) to globals.new($asdf$,[123]) and asdf.qwerty to globals.arr($asdf$,"querty") and all function declarations to a freeze function in that case it would be:

Quote

globals.fn((globals.array([]),globals.arr(globals.new(globals.freeze(function(){
this.z=globals.freeze(function(){
// this==window
})}))),"z"),[]);

and then check in all those functions if [this] and [arguments] are instance of your globals prototypes (globals.array,number,etc..) so you'll be sure the user is never getting a reference he didnt created.

hardcore to bypass now eh?

--------------------------------
http://sirdarckcat.blogspot.com/ http://www.sirdarckcat.net/ http://foro.elhacker.net/ http://twitter.com/sirdarckcat



Edited 8 time(s). Last edit at 07/01/2009 12:34AM by sirdarckcat.

@sirdarckcat

Thanks! Nice vector I like it, I shall work on a fix for this and sort out scoping issues

Update...
Ok this is fixed for now. I've added symbols which are locally scoped not on the window. This isn't a complete fix yet as I plan to rewrite functions objects etc when I have time. JSReg now includes a eval'd box and Function box to see the rewritten code when using globals.eval and globals.Function

New update...
I've fixed the for..in problems. You should be able to do a for in loop without hacks :) I've made the statements regexps stronger now before it wasn't checking boundaries which could leak variables to window like:-
varvar = function(){} // window.varvar

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]



Edited 2 time(s). Last edit at 07/05/2009 11:54AM by Gareth Heyes.

JSReg now supports:-

globals.setTimeout("globals.eval('globals.alert(1)')",500);

xy=1;globals.eval("xy");

x=globals.Function("return 123");x();

Can you hack it?

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

Why does this return `undefined`?

(function(){ return function(){}; })()();

@kangax

Because your inner function returns nothing. Glad to see you on here :)

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

It does return nothing indeed : ) Strange, I could have sworn that inner function was somehow tripping up: working as expected when invoked inside wrapping function, but not when overall "outside" expression was called.

I also noticed that this blows up:

(/**/2+2/**/); // `SyntaxError`, should be `4`

@kangax

Awesome find thanks! This is useful for me I'll fix this. Must be a bug in the comment regexps

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

@kangax

Fixed. It was a greedy regexp problem

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

@Gareth
Cool. Here's another one for you :)

1/1/1
expected: 1
actual: SyntaxError

Quote

Quote

@Gareth
Cool. Here's another one for you :)

1/1/1
expected: 1
actual: SyntaxError

suggested patch:

		regexps = new RegExp(endStatement.source+"?"+spaces.source+"(?:[\\/](?:\\\\[\\/]|[^\\/*])(?:\\\\[\\/]|[^\\/])*[\\/](?:[gmi]*))"+spaces.source),

also, this is wrong, isnt it?

({a:b})

gets converted to

var $a$,$b$;
({$a$:$b$})

why var $a$?

I've been checking the code of addSymbol:function(symbol) {, is there a particular reason you are not using maps instead of arrays? It would make the parsing faster and easier.

It would be something like this

Quote

addSymbol:function(symbol) {
			if(!new RegExp('^[$]'+variable.source+'[$]$').test(symbol)) {
				error('Parser error:Invalid symbol');
			}
			if (typeof this.symbolsMap[symbol]=="undefined") {
				this.symbols.push(symbol);
				this.symbolsMap[symbol]=true;
			}
}

and

Quote

		init:function() {
			this.symbols = [];
			this.symbolsMap = {};
		},

and the same idea would apply with extractSymbols.

Quote

		extractSymbols:function(code, compare) {
			if (this.symbols.length) {
				var symbols = this.symbols;						
				if(compare) {
					symbols = [];
					for(var i=0;i<compare.length;i++) {
						if(typeof this.symbolsMap[compare] != "undefined") {
							symbols.push(compare);
						}
					}	
				}						
				if (symbols.length) {
					code = 'var ' + symbols.join(',') + ';\n' + code;
				}
			}
			return code;
		},	

reserved words

Quote

original

(function(){
if(true){a=1}
})();
globals.alert(a);

Quote

modified

var $true$,$a$;
(function(){
if($true$){$a$=globals.number(1)}
})();
globals.$alert$($a$);

fixing this using the maps approach is easy, since you wont need to remove them, just add them to the map but not to the array:

Quote

		init:function() {
			this.symbols = [];
			this.symbolsMap = {'$true$':1,'$false$':1,'$null$':1,'$break$':1,'$case$':1,'$catch$':1,'$continue$':1,'$default$':1,'$delete$':1,'$do$':1,'$else$':1,'$finally$':1,'$for$':1,'$function$':1,'$if$':1,'$in$':1,'$instanceof$':1,'$new$':1,'$return$':1,'$switch$':1,'$this$':1,'$throw$':1,'$try$':1,'$typeof$':1,'$var$':1,'$void$':1,'$while$':1,'$with$':1};
		},

then maybe here:

Quote

							if($objects.replace(/[\s\n]/g,'') == 'this' && $objects.length > 1) {
								$objects = 'this';
								continue;
							}

you could do:

Quote

							var t;
							if(self.symbolsMap[t=$objects.replace(/[\s\n]/g,'')]===1 && $objects.length > 1) {
								$objects = t;
								continue;
							}

and

Quote

							if(i == $objects.length -1 && allowedProperties.test($objects.replace(/[\s\n]/g,'')) && typeof self.symbolsMap[$objects.replace(/[\s\n]/g,'')]=='undefined') {										
								$objects = $objects.replace(/[\s\n]/g,'');
								continue;
							}	

and well, wrap the objects in your safe natives or etc :P

could you put jsreg in a js file plz?

Greetz!!

--edit--
lol:
Edited 23 time(s). Last edit at 07/14/2009 12:33AM by sirdarckcat.

lots of edits xD

--------------------------------
http://sirdarckcat.blogspot.com/ http://www.sirdarckcat.net/ http://foro.elhacker.net/ http://twitter.com/sirdarckcat



Edited 23 time(s). Last edit at 07/14/2009 01:33AM by sirdarckcat.

@kangax @sirdarckcat

Awesome feedback! These are great points and I'll fix them all. Sdc I'll put it in a js file when these are fixed.

Thanks!!!!

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

Quote

@Gareth
Cool. Here's another one for you :)
1/1/1
expected: 1
actual: SyntaxError

Ok this is fixed. I've updated the RegExp for the regexp object. Very tricky syntax :) Thanks kangax and thanks sirdarckcat it's pretty much your patch

Quote

also, this is wrong, isnt it?

({a:b})

gets converted to

var $a$,$b$;
({$a$:$b$})

why var $a$?

Yeah this is a bug, at the moment JSReg has no way to identify the object context so it thinks it's a variable or label.

Quote

I've been checking the code of addSymbol:function(symbol) {, is there a particular reason you are not using maps instead of arrays? It would make the parsing faster and easier.

It's a valid point, the code you supplied made more sense and is faster. I've added it. The only reason I did it this way was paranoia, lets say you could add a malicious symbol at was __parent__ or something. My regexp does check it twice though and I return the function as well :)

Quote

reserved words

Quote:
original

(function(){
if(true){a=1}
})();
globals.alert(a);

Quote:
modified

var $true$,$a$;
(function(){
if($true$){$a$=globals.number(1)}
})();
globals.$alert$($a$);

Yep this was a bug. I have a separate RegExp for js keywords and I've added them to the list. Eventually I'll rewrite the code for them too

Quote

could you put jsreg in a js file plz?

Yep this is done. I've separated the HTML and the JS. You can define your own debug textareas now too using setDebugObjects

Big thanks to both of you again!

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]



Edited 1 time(s). Last edit at 07/14/2009 10:06AM by Gareth Heyes.

This - `(x = /1/); /1/` - results in - `($x$= /globals.number(1)/)globals.regexp(; /1/)` and throws SyntaxError. Should evaluate to last statement of course - /1/. Also note how first regex is parsed as `/globals.number(1)/` and not as `globals.regexp(/1/)`.

Cheers.

@kangax

Thanks good find, damn these regexp/numbers are tricky :)

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

New JSReg is up. There's a bug in IE atm but it works in FF.

@kangax

Should have fixed that tricky one now! Hopefully! Thanks again

We now have DOS protection! :-
while(true)globals.alert(1);

The is a max limit for function calls and loops. I doubt there won't be a bug here as I had to write a lot of regexps for them =)

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

/a//**///b/

Expected: /a/
Actual: /a//** (SyntaxError)

Also, `delete` and `void` are not supported, are they?

delete ({ x: 1 }).x; // ReferenceError
void 1; // ReferenceError

they are not supported afaik.

gareth, I think that for the loop rewrite, the loop0: marker shouldn't be putted (actually, I think you should copy user's loop markers), and the if/else injection should take place before the {} in the loop:

Quote

for(1;1;1){break}

should be:

Quote

;var loop0=0;(globals.number(1));while(true)if(general.checkMaxLoop()&&((loop0++>0&&globals.number(1))||true)&&(!(globals.number(1))))break; else {break}

and

Quote

for(1;1;1)a()

should be

Quote

;var loop0=0;(globals.number(1));while(true)if(general.checkMaxLoop()&&((loop0++>0&&globals.number(1))||true)&&(!(globals.number(1))))break; else a()

and

Quote

for(1;1;1);

should be:

Quote

;var loop0=0;(globals.number(1));while(true)if(general.checkMaxLoop()&&((loop0++>0&&globals.number(1))||true)&&(!(globals.number(1))))break; else{}

Also, there's a bug on empty fors,

Quote

for(;;);

gets converted to:

Quote

;var loop0=0;loop0:while(true)if(general.checkMaxLoop();

which is a syntax error =/

Greetz!!

--------------------------------
http://sirdarckcat.blogspot.com/ http://www.sirdarckcat.net/ http://foro.elhacker.net/ http://twitter.com/sirdarckcat



Edited 1 time(s). Last edit at 07/16/2009 11:37PM by sirdarckcat.

@kangax

Added void, I'm not supporting delete.
Thanks for another cool bug report!!! I'll fix the comment regexps stuff

@sirdarckcat

Thanks man! I'll fix this too

....In the meantime JSReg now parses unicode and removes the need to call globals.alert, alert(1) will work fine :D

This works in JSReg:-
a='a';eval(\u0061+'\x6c\x65\x72\x74\x28\u0034\x32\u0029');

Oh I forgot to mention JSReg now works correctly in IE and FF2 :D
It even works in IE5.5 with my tests!!!! :o

Update......
@sirdarckcat

1. Markers no longer added. Labels can be added by the user and are rewritten $$
2. Blank fors are allowed now
3. {} are only added when the user adds them

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]



Edited 2 time(s). Last edit at 07/17/2009 08:36AM by Gareth Heyes.

Gareth,

You turn `alert(...)`, which is an expression into `if (...)` which is a statement, so once `alert` occurs as part of another expression, the whole thing results in a SyntaxError:

(alert(1)); // SyntaxError

Some more that result in a SyntaxError, but shouldn't.

/x/[1];

var arr = [1,2,[']']];

try { "catch(){}"; } catch(e) { }

yep he is right, the checkmaxfunctioncalls should be done inside the function IMHO

good job with unicode!!

--------------------------------
http://sirdarckcat.blogspot.com/ http://www.sirdarckcat.net/ http://foro.elhacker.net/ http://twitter.com/sirdarckcat

@kangax @sirdarckcat

Agreed I was just being lazy =) I'll rewrite function definitions.

@kangax

Fixed all those apart from the RegExp one. That one is tricky again because of conflicts with comments and operators. I'll find a way though.

Update...
Ok fixed the Regexp one too /a/[1]
I had to give in and use positive lookaheads, this won't work in browsers using < Javascript 1.5

Update again...
Added a separate RegExp for function statements and the recursion check is added before the rest of the function body

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]



Edited 4 time(s). Last edit at 07/18/2009 03:34PM by Gareth Heyes.

@Gareth

Array initializer still seems to be glitchy:

[ 1,2,[']',']'] ]; // SyntaxError

And another unexpected SyntaxError when evaluating - `({ "x": 1 })`

@kangax

Great catch! Small bug in the array rewriter wasn't trimming spaces.

I've added a regexp to detect a string followed by a : to identify object properties. Thanks again! JSReg is getting better and better thanks to you guys!

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

[1,2,3][1];

expected: 2
actual: undefined

Also, non-string keys still don't work:

({ 1: 1 }); // SyntaxError