fix(security): do not auto-bootstrap when loaded from an extension. #15346

mprobst · 2016-11-01T18:15:21Z

Extension URIs (resource://...) bypass Content-Security-Policy in Chrome and
Firefox and can always be loaded. Now if a site already has a XSS bug, and uses
CSP to protect itself, but the user has an extension installed that uses
Angular, an attacked can load Angular from the extension, and Angular's
auto-bootstrapping can be used to bypass the victim site's CSP protection.

Notes:

isAutoBootstrapAllowed must be initialized on load, so that currentScript
is set correctly.
The tests are a bit indirect as reproducing the actual scenario is too
complicated to reproduce (requires signing an extension etc). I have confirmed
this to be working manually.

mprobst · 2016-11-01T18:17:42Z

/CC @rjamet @koto

koto · 2016-11-01T19:03:45Z

LGTM

gkalpak · 2016-11-01T19:59:59Z

src/Angular.js

@@ -1602,6 +1622,11 @@ function angularInit(element, bootstrap) {
    }
  });
  if (appElement) {
+    if (!isAutoBootstrapAllowed) {
+      console.error('Angular: disabling automatic bootstrap. <script> protocol indicates an ' +
+          'extension, document.href does not match.');


document.location.href?

gkalpak · 2016-11-01T20:23:52Z

src/Angular.js

@@ -1602,6 +1622,11 @@ function angularInit(element, bootstrap) {
    }
  });
  if (appElement) {
+    if (!isAutoBootstrapAllowed) {


This check could be moved at the top (unless we expect anyone to load angular via resource:/chrome-extension: in a non-extention page and bootstrap it manually.

I didn't do this intentionally - otherwise you'll get an error message even if you intend to manually bootstrap by calling angular.bootstrap. Which I guess is a legit use case, if somebody does inject Angular in the page but also controls it to call angular.bootstrap (not a security issue because they can already execute JS in the page in that case).

Yeah, I thought it might be intentional. I don't expect anyone to be loading angular over resource: on non-extension pages, but you never know 😃

Hm...so, if it is a supported usecase (loading angular.js over resource: on non-extension pages), isn't this chage breaking? In that case, we should have a breaking change notice.

Loading angular.js over resource: from a non-extension page is not supported, and not a use case (that's almost certainly a security exploit, which is why this change is blocking it). I don't think we need to document that.

However, for the rare case where a web page really actually wants to load via resource://, it still can, by embedding an appropriate angular.bootstrap call. If you can embed JS into the hosting page, you can do evil things anyway, so that's not a security boundary. This behaviour is easy to implement, doesn't do harm or lower security (it should, at least...), and overall seems fine.

A secondary effect is that the console spam only triggers if the user actually has an ng-app in their app, so it's more precise, which I think is a good thing.

So I don't think we need to document anything. Most users will never notice this.

So I don't think we need to document anything. Most users will never notice this.

This should be documented, assuming that most users won't notice it means that when someone does notice it, there is no help available on how to go about making it work.

@mmccallum - feel free to send in a PR.

@mmccallum FWIW, when auto bootstrap gets disabled, Angular prints out a pretty clear message in the console.

Extension URIs (`resource://...`) bypass Content-Security-Policy in Chrome and Firefox and can always be loaded. Now if a site already has a XSS bug, and uses CSP to protect itself, but the user has an extension installed that uses Angular, an attacked can load Angular from the extension, and Angular's auto-bootstrapping can be used to bypass the victim site's CSP protection. Notes: - `isAutoBootstrapAllowed` must be initialized on load, so that `currentScript` is set correctly. - The tests are a bit indirect as reproducing the actual scenario is too complicated to reproduce (requires signing an extension etc). I have confirmed this to be working manually.

gkalpak · 2016-11-01T21:19:26Z

src/Angular.js

@@ -97,6 +97,9 @@
  NODE_TYPE_DOCUMENT,
  NODE_TYPE_DOCUMENT_FRAGMENT
 */
+/* global
+  console


You can avoid this by accessing console off window (not sure what is better).

gkalpak · 2016-11-01T21:23:11Z

src/Angular.js

@@ -1602,6 +1622,11 @@ function angularInit(element, bootstrap) {
    }
  });
  if (appElement) {
+    if (!isAutoBootstrapAllowed) {


Yeah, I thought it might be intentional. I don't expect anyone to be loading angular over resource: on non-extension pages, but you never know 😃

Hm...so, if it is a supported usecase (loading angular.js over resource: on non-extension pages), isn't this chage breaking? In that case, we should have a breaking change notice.

petebacondarwin · 2016-11-02T11:02:56Z

src/Angular.js

+  var docLoadProtocol = document.location.protocol;
+  if ((scriptProtocol === 'resource:' ||
+       scriptProtocol === 'chrome-extension:') &&
+      docLoadProtocol !== scriptProtocol) {


So we do allow auto bootstrap if the protocol of the document matches that of the script?
Why is that?

Because that's the intended use case - an extension that has a page (within the extension) that loads angular, and contains ng-app. We could disable that, but if we support auto bootstrap, I think we should support it within extensions, shouldn't we?

petebacondarwin · 2016-11-02T11:23:28Z

src/Angular.js

@@ -1602,6 +1622,11 @@ function angularInit(element, bootstrap) {
    }
  });
  if (appElement) {
+    if (!isAutoBootstrapAllowed) {
+      window.console.error('Angular: disabling automatic bootstrap. <script> protocol indicates ' +
+          'an extension, document.location.href does not match.');


I feel that this message could be clearer or that it should link to a webpage/issue that explains what is happening?

Do you have a suggestion? This should really only happen when somebody tries to exploit the security hole, so I'm not too worried about usability here. I don't think we need docs or a website.

But it shouldn't block merging this.

Extension URIs (`resource://...`) bypass Content-Security-Policy in Chrome and Firefox and can always be loaded. Now if a site already has a XSS bug, and uses CSP to protect itself, but the user has an extension installed that uses Angular, an attacked can load Angular from the extension, and Angular's auto-bootstrapping can be used to bypass the victim site's CSP protection. Notes: - `isAutoBootstrapAllowed` must be initialized on load, so that `currentScript` is set correctly. - The tests are a bit indirect as reproducing the actual scenario is too complicated to reproduce (requires signing an extension etc). I have confirmed this to be working manually. Closes #15346

Extension URIs (`resource://...`) bypass Content-Security-Policy in Chrome and Firefox and can always be loaded. Now if a site already has a XSS bug, and uses CSP to protect itself, but the user has an extension installed that uses Angular, an attacked can load Angular from the extension, and Angular's auto-bootstrapping can be used to bypass the victim site's CSP protection. Notes: - `isAutoBootstrapAllowed` must be initialized on load, so that `currentScript` is set correctly. - The tests are a bit indirect as reproducing the actual scenario is too complicated to reproduce (requires signing an extension etc). I have confirmed this to be working manually. Closes angular#15346

mprobst assigned petebacondarwin Nov 1, 2016

googlebot added the cla: yes label Nov 1, 2016

gkalpak reviewed Nov 1, 2016

View reviewed changes

gkalpak modified the milestones: Backlog, 1.5.9 Nov 1, 2016

gkalpak added type: feature frequency: low component: misc core severity: security labels Nov 1, 2016

mprobst force-pushed the protocols-resources-bootstrap branch from b5e6da4 to ca88480 Compare November 1, 2016 20:49

gkalpak reviewed Nov 1, 2016

View reviewed changes

chore(): fix global console usage.

c2364d2

petebacondarwin reviewed Nov 2, 2016

View reviewed changes

petebacondarwin closed this in 0ff10e1 Nov 2, 2016

IgorMinar mentioned this pull request Nov 24, 2016

Angular 1.5.8 Ban mozilla/addons-linter#1000

Closed

This was referenced Mar 19, 2017

[DevAudit] [audit] audit on 3/19/2017 11:51 PM allisterb/DevAuditReporterTest#1

Open

[DevAudit] [audit] audit on 3/19/2017 11:53 PM sonatype-nexus-community/DevAudit#64

Closed

OSSIndex-Admin mentioned this pull request Mar 20, 2017

[DevAudit] [audit] audit on 3/20/2017 3:40 AM allisterb/DevAuditReporterTest#2

Open

This was referenced Oct 6, 2020

WS-2017-0113 (High) detected in angular-1.5.8.min.js tt9133github/zheng#72

Open

WS-2017-0113 (High) detected in angular-1.5.0.js Watemlifts/redash#54

Open

mend-for-github-com bot mentioned this pull request Oct 9, 2020

WS-2017-0113 (High) detected in angular-1.5.0.js kenferrara/hadoop#35

Open

mend-for-github-com bot mentioned this pull request Oct 9, 2020

WS-2017-0113 (High) detected in angular-1.5.0.js robertjfinn/hadoop#40

Open

This was referenced Oct 19, 2020

WS-2017-0113 (High) detected in angular-1.5.5.js, angular-1.5.3.js olivialancaster/angular#4

Open

WS-2017-0113 (High) detected in angular-1.5.5.js, angular-1.5.3.js gate5/angular#3

Open

mend-for-github-com bot mentioned this pull request Oct 21, 2020

WS-2017-0113 (High) detected in angular-1.5.0.js LevyForchh/t-vault#25

Open

mend-for-github-com bot mentioned this pull request Nov 23, 2020

WS-2017-0113 (High) detected in angular-1.5.6.min.js LevyForchh/octobot#7

Open

mend-bolt-for-github bot mentioned this pull request Nov 24, 2020

WS-2017-0113 (High) detected in angular-1.5.0.js, angular-1.5.0.min.js dundermifflin0/struts-examples#21

Open

mend-for-github-com bot mentioned this pull request Dec 18, 2020

WS-2017-0113 (High) detected in angular-1.5.0.js billmcchesney1/t-vault#106

Open

mend-bolt-for-github bot mentioned this pull request Dec 22, 2020

WS-2017-0113 (High) detected in angular-1.5.6.min.js valdisiljuconoks/localization-provider-aspnet#5

Open

mend-bolt-for-github bot mentioned this pull request Jan 20, 2021

WS-2017-0113 (High) detected in angular-1.5.5.js, angular-1.5.3.js turkdevops/angular#112

Open

This was referenced Apr 6, 2021

WS-2017-0113 (High) detected in multiple libraries - autoclosed VivekBuzruk/Hygieia#232

Closed

WS-2017-0113 (High) detected in multiple libraries - autoclosed VivekBuzruk/Hygieia#252

Closed

mend-for-github-com bot mentioned this pull request Jun 30, 2021

WS-2017-0113 (High) detected in angular-1.5.5.js, angular-1.5.3.js joshnewton31080/angular#51

Open

mend-bolt-for-github bot mentioned this pull request Sep 5, 2021

WS-2017-0113 (High) detected in angular-1.5.5.min.js turkdevops/play-with-docker#11

Open

mend-bolt-for-github bot mentioned this pull request Oct 5, 2021

WS-2017-0113 (High) detected in angular-1.5.8.js, angular-v1.5.8 - autoclosed bsbtd/Teste#745

Closed

mend-bolt-for-github bot mentioned this pull request Oct 20, 2021

WS-2017-0113 (High) detected in angular-1.5.0.js rlennon/whitesource_bolt_demo#64

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): do not auto-bootstrap when loaded from an extension. #15346

fix(security): do not auto-bootstrap when loaded from an extension. #15346

mprobst commented Nov 1, 2016

mprobst commented Nov 1, 2016

koto commented Nov 1, 2016

gkalpak Nov 1, 2016

mprobst Nov 1, 2016

gkalpak Nov 1, 2016

mprobst Nov 1, 2016

gkalpak Nov 1, 2016

mprobst Nov 2, 2016

mmccallum Jan 8, 2017

petebacondarwin Jan 8, 2017

mprobst Jan 9, 2017

gkalpak Nov 1, 2016

gkalpak Nov 1, 2016

petebacondarwin Nov 2, 2016

mprobst Nov 2, 2016

petebacondarwin Nov 2, 2016

mprobst Nov 2, 2016

petebacondarwin Nov 2, 2016

fix(security): do not auto-bootstrap when loaded from an extension. #15346

fix(security): do not auto-bootstrap when loaded from an extension. #15346

Conversation

mprobst commented Nov 1, 2016

mprobst commented Nov 1, 2016

koto commented Nov 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment