17

I found a bug that allows you to escape the AngularJS template sandbox. Angular is a mustache based template language. It allows you to put expressions that are evaluated in your html. For example, {{1+1}} renders at 2

The sandbox makes it so users can't access window / constructors from inside Angular within expressions. You can't do {{{}.constructor = ...}} for example. This is because those operations are considered unsafe should the site also be saving templates from user input. It opens up the site to XSS.

Going through the source code, I found that there is a check for obj === {}.constructor that can be bypassed.

Basically, it comes down to bypassing this:

if (obj) {
    if (obj === (0).constructor || obj === (false).constructor || obj ===     ''.constructor ||
      obj === {}.constructor || obj === [].constructor || obj ===     Function.constructor) {
      throw $parseMinErr('isecaf',
      'Assigning to a constructor is disallowed! Expression: {0}',      fullExpression);
    }
}

I was able to get past those checks by hiding my constructor calls inside another object (object literal or array literal).

As literal object:

{{x = {'y':''.constructor.prototype}; x['y'].charAt=[].join;$eval('x=alert(`Evaluated Object Literal`)');}}

Or as an array:

{{x = [''.constructor.prototype]; x[0].charAt=[].join; $eval('x=alert(`Evaluated Array`)');}}

How can the above checks be best improved to make it so these cases fail?

I tried changing the obj === {}.constructor checks to instanceof instead and that makes the checks work, but I think it may introduce its own security issues. I would like to submit a pull request, but I'm hoping other security researchers out there might contribute to the conversation in order to make the sandbox as strong as possible.

Here is my issue posted to Angular project for reference: https://github.com/angular/angular.js/issues/14939

Here is related material: http://blog.portswigger.net/2016/01/xss-without-html-client-side-template.html

Here is a jsFiddle: https://jsfiddle.net/ianhickey/5sb665we/

sven.to
  • 586
  • 3
  • 5
ialexander
  • 311
  • 2
  • 9
  • Might be worth tweeting to @ garethheyes and @ albinowax as they've found several Angular [sandbox escapes](http://blog.portswigger.net/2016/01/xss-without-html-client-side-template.html) – paj28 Jul 22 '16 at 13:53
  • to secure, simply: `delete Object.prototype.constructor;`, which stops such attacks in `"use strict"` (its impossible to stop otherwise afaik). note that you don't even need a temp stand-in in core JS: `constructor.constructor==Function`, since `window` inherits from `Object.prototype`, and lexical scope inherits from global... Might consider the other measures taken by http://danml.com/js/subeval.js – dandavis Jul 22 '16 at 16:26
  • @paj28 I posted a link to the issue on Portswigger in case they want to weight in. Thanks for the suggestion. – ialexander Jul 22 '16 at 22:14
  • @dandavis I think that deleting Object.prototype.constructor could break other Angular functionality but I'm going to try it and run the Angular tests to check. Thanks for the link to subeval. – ialexander Jul 22 '16 at 22:14
  • I don't quite understand the point. The sanitation of Angular expressions is only designed to prevent common unsafe/confusing usages and not meant to be a complete security sandbox[\[1\]](https://docs.angularjs.org/guide/security#expression-sandboxing). To evaluate these expressions the attacker would need to be able to change HTML source code, which already gives way more power than these expressions. (A badly written site can pass user input to `$interpolate` but that's already own-foot-shooting scenario anyways.) – billc.cn Jul 28 '16 at 14:26
  • 1
    @billc.cn The the top of the $parse source file states that modifications should not allow arbitrary javascript execution. Currently, it does. That's all I'm saying: Changes to this file can potentially create security vulnerabilities. An approval from 2 Core members with history of modifying this file is required. Does the change somehow allow for arbitrary javascript to be executed? Or gives undesired access to variables likes document or window? – ialexander Jul 30 '16 at 15:44
  • The issue seems to be fixed and closed now. I encourage you to answer your own question and accept it :) – GnP Aug 29 '16 at 17:25
  • 1
    AngularJS are now [removing the sandbox](http://angularjs.blogspot.co.uk/2016/09/angular-16-expression-sandbox-removal.html) – paj28 Sep 08 '16 at 14:31

1 Answers1

4

Angular users: Stop Saving UNSANITIZED User Input.

I have mixed feelings about the answer to my own question. As of Angular version 1.6, the Angular team has decided to remove the sandbox altogether. The sandbox (while not perfect) afforded the user some superficial level of protection against there own choices when building an Angular app. The sandbox was really pretty good and getting better and better. But the sandbox was also never intended to be a security boundary. It was there to try and help enforce clean design principals.

With the sandbox gone, the Angular user is going to be forced to think about the security implications of saving untrusted, un-sanitized, user input. Which I personally think is a good thing. If, and only if, people realize going into that upgrade that it could potentially open wider security issues that they had previously ignored. Hopefully this post helps create some awareness when people are searching 'angular 1.6 security'.

The answer to this question is that Angular has removed the sandbox as of 1.6 to address the fact that users were using the sandbox as a security mechanism where it was never intended to be.

Angular Sandbox Removal Blog post

ialexander
  • 311
  • 2
  • 9