Sandbox allows access to constructor versions 1.3.3+ - Bypassing the sandbox.

[ian-a-hickey]

Note to Google AngularJS:
Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
The sandbox (while not a security feature) is supposed to error on access to constructor properties. In its current state it is trivial to bypass.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

https://jsfiddle.net/ianhickey/5sb665we/

Sandbox escape from 1.3.2 fixed in 1.3.3 by Gareth Heyes (PortSwigger):
'a'.constructor.prototype.charAt=[].join;$eval('x=alert(1)');

My versions modified to work with 1.3.3 - 1.5.7+:
As literal object: {{x = {'y':''.constructor.prototype}; x['y'].charAt=[].join;$eval('x=alert(Evaluated Object Literal)');}}
OR:
As Array: {{x = [''.constructor.prototype]; x[0].charAt=[].join; $eval('x=alert(Evaluated Array)');}}

In these examples I am able to mask what I'm doing and the expression passes the ensureSafeAssignContext checks.

What is the expected behavior?
The expected behavior is that the sandbox would throw an 'Assigning to constructor is disallowed' error.

What is the motivation / use case for changing the behavior?
The motivation here is to prevent trivial sandbox escapes and help protect users from template injection. While Google / AngularJS don't consider the sandbox a security boundary I think it benefits users to have the strongest possible sandbox. I think its fair to say that many AngularJS consumers are not utilizing the tools Angular provides for security. Which is really the status-quo across open-source projects. Fixing this would make the sandbox stronger.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

This bug effects version 1.3.3 to 1.5.7+. I’ve only tested it on 1.3.3, 1.5.5 and 1.5.7.

The faulty logic seems to be in ensureSafeAssignContext in $parse.

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);
}
}

which fails in the above examples.

I would like to submit a pull request to fix the issue if that will be allowed. I know this is a sensitive part of the source-code that can't be modified by just anyone.

[gkalpak]

@ian-a-hickey, everyone is allowed to submit pull requests against any part of the code. Getting it merged is a different story 😛
If you do submit a PR with a fix, I am sure someone will give it a review, so feel free to go ahead 😉

[ian-a-hickey]

Ah, good to know. I'm going through the best practices for pull requests right now. I'll touch base again once I push my code to discuss the relative merits of the fixes.

[petebacondarwin]

OK, so, while this is a bug in the Angular sandbox, this is not actually a security issue, so I have changed the label. See https://docs.angularjs.org/guide/security#expression-sandboxing

[ian-a-hickey]

That was a quick fix! Maybe I'll have time to contribute next time. Cheers.

[gkalpak]

Sorry! I happened to be in the $parse neighborhood a couple of days later (looking into something else), so... 😁

[gkalpak]

@ian-a-hickey, would you like looking into #14990. I took your issue, you can take mine (if you want to) 😃