Remove eval and Function from build to support strict CSP #6872

Open
saulshanabrook opened this Issue Jul 3, 2017 · 13 comments

Comments

Projects
None yet

saulshanabrook commented Jul 3, 2017

Bug Report or Feature Request (mark with an x)

- [ ] bug report -> please search issues before submitting
- [x] feature request

Versions.

@angular/cli: 1.2.0
node: 8.1.0
os: darwin x64
@angular/animations: 4.2.5
@angular/common: 4.2.5
@angular/compiler: 4.2.5
@angular/core: 4.2.5
@angular/forms: 4.2.5
@angular/http: 4.2.5
@angular/platform-browser: 4.2.5
@angular/platform-browser-dynamic: 4.2.5
@angular/router: 4.2.5
@angular/cli: 1.2.0
@angular/compiler-cli: 4.2.5
@angular/language-service: 4.2.5

Repro steps.

$ ng new test-csp
$ yarn add web-ext

Create src/manifest.json with:

{
  "manifest_version": 2,
  "name": "test-csp",
  "version": "1.0",
  "browser_action": {
    "default_title": "test-csp",
    "default_popup": "/index.html"
  }
}

Add manifest.json to the apps[0].assets in the .angular-cli.json.

Repo available here https://github.com/saulshanabrook/test-csp

The log given by the failure.

Regular build:

$ ng build
$ ./node_modules/.bin/web-ext lint -s dist
Validation Summary:

errors          0
notices         0
warnings        11

WARNINGS:

Code                    Message                          Description                                                     File                  Line    Column
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.bundle.js   286     22
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.bundle.js   850     71
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.bundle.js   2397    11
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          eval can be harmful.             Evaluation of strings as code can lead to security              polyfills.bundle.js   2397    43
                                                         vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.bundle.js      2576    11
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          eval can be harmful.             Evaluation of strings as code can lead to security              vendor.bundle.js      2576    43
                                                         vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.bundle.js      36267   73
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.bundle.js      36273   17
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.bundle.js      36281   17
                        eval.                            vulnerabilities and performance issues, even in the most
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML   Due to both security and performance concerns, this may not     vendor.bundle.js      50833   71
                                                         be set using dynamic values which have not been adequately
                                                         sanitized. This can lead to security issues or fairly serious
                                                         performance degradation.
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML   Due to both security and performance concerns, this may not     vendor.bundle.js      50878   9
                                                         be set using dynamic values which have not been adequately
                                                         sanitized. This can lead to security issues or fairly serious
                                                         performance degradation.

Production build:

$ ng build --prod
$ ./node_modules/.bin/web-ext lint -s dist
Validation Summary:

errors          0
notices         0
warnings        11

WARNINGS:

Code                    Message                          Description                                                     File                              Line   Column
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.35d6fc6174fa08d451d6…   1      52746
                        eval.                            vulnerabilities and performance issues, even in the most        .bundle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          eval can be harmful.             Evaluation of strings as code can lead to security              polyfills.35d6fc6174fa08d451d6…   1      52776
                                                         vulnerabilities and performance issues, even in the most        .bundle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.35d6fc6174fa08d451d6…   1      55491
                        eval.                            vulnerabilities and performance issues, even in the most        .bundle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              polyfills.35d6fc6174fa08d451d6…   1      56434
                        eval.                            vulnerabilities and performance issues, even in the most        .bundle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.7157c5dcd45d72de6187.bu…   1      54515
                        eval.                            vulnerabilities and performance issues, even in the most        ndle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.7157c5dcd45d72de6187.bu…   1      54590
                        eval.                            vulnerabilities and performance issues, even in the most        ndle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.7157c5dcd45d72de6187.bu…   1      54778
                        eval.                            vulnerabilities and performance issues, even in the most        ndle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is      Evaluation of strings as code can lead to security              vendor.7157c5dcd45d72de6187.bu…   1      127089
                        eval.                            vulnerabilities and performance issues, even in the most        ndle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
DANGEROUS_EVAL          eval can be harmful.             Evaluation of strings as code can lead to security              vendor.7157c5dcd45d72de6187.bu…   1      127119
                                                         vulnerabilities and performance issues, even in the most        ndle.js
                                                         innocuous of circumstances. Please avoid using `eval` and the
                                                         `Function` constructor when at all possible.'
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML   Due to both security and performance concerns, this may not     vendor.7157c5dcd45d72de6187.bu…   1      141593
                                                         be set using dynamic values which have not been adequately      ndle.js
                                                         sanitized. This can lead to security issues or fairly serious
                                                         performance degradation.
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML   Due to both security and performance concerns, this may not     vendor.7157c5dcd45d72de6187.bu…   1      142070
                                                         be set using dynamic values which have not been adequately      ndle.js
                                                         sanitized. This can lead to security issues or fairly serious
                                                         performance degradation.

Desired functionality.

We should be able to build the project with no CSP errors. That means eliminating all uses of eval and Function. This is needed in order for Mozilla to let any Angular 2 apps be submitted as extensions on their store. For example, I got this response when trying to submit an extension:

This version didn't pass review because of the following problems:

  1. 'unsafe-eval' usage into Content Security Policy.

We generally don't accept using the 'eval' function. There are many reasons not to use 'eval', and there are alternatives available. You can read more about it here: https://developer.mozilla.org/en/XUL_School/Appendix_C:_Avoid_using_eval_in_Add-ons

Mention any other details that might be useful.

Moved from #1279 (comment).

This might be blocked on Angular core angular/angular#6361 angular/angular#1744.

This comment has been minimized.

Show comment Hide comment
@filipesilva

filipesilva Jul 17, 2017

Member

There are, unfortunately, some eval usage right now that I don't know if we can easily get rid of. One such example is using script-loader to load scripts. This needs some investigation to see what can be done and where are the offending bits.

Member

filipesilva commented Jul 17, 2017

There are, unfortunately, some eval usage right now that I don't know if we can easily get rid of. One such example is using script-loader to load scripts. This needs some investigation to see what can be done and where are the offending bits.

This comment has been minimized.

Show comment Hide comment
@justingrayston

justingrayston Aug 3, 2017

Contributor

For me, the report is coming from webpack code trying to load the modules.

e.exports=function(e){"undefined"!=typeof execScript?execScript(e):eval.call(null,e)}}

Which I believe is the compiled version of

/*
    MIT License http://www.opensource.org/licenses/mit-license.php
    Author Tobias Koppers @sokra
*/
module.exports = function(src) {
    if (typeof execScript !== "undefined")
        execScript(src);
    else
        eval.call(null, src);
}

This issue in Webpack webpack/webpack#4094 kind of implies that it should be possible to resolve, but to be honest I am no webpack expert. If I have some time I may do some reading.

Contributor

justingrayston commented Aug 3, 2017

For me, the report is coming from webpack code trying to load the modules.

e.exports=function(e){"undefined"!=typeof execScript?execScript(e):eval.call(null,e)}}

Which I believe is the compiled version of

/*
    MIT License http://www.opensource.org/licenses/mit-license.php
    Author Tobias Koppers @sokra
*/
module.exports = function(src) {
    if (typeof execScript !== "undefined")
        execScript(src);
    else
        eval.call(null, src);
}

This issue in Webpack webpack/webpack#4094 kind of implies that it should be possible to resolve, but to be honest I am no webpack expert. If I have some time I may do some reading.

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Aug 15, 2017

feat(@angular/cli): support sourcemaps and minification in scripts
Adds sourcemap and minification to javascript added via the `scripts` array in `.angular-cli.json`.

`script-loader` is no longer used, which should help with CSP since it used `eval`.

Scripts will no longer appear in the console output for `ng build`, as they are now assets instead of webpack entry points.

It's no longer possible to have the `output` property of both a `scripts` and a `styles` entry pointing to the same file. This wasn't officially supported or listed in the docs, but used to be possible.

Fix #2796
Fix #7226
Fix #7290

Related to #6872

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Aug 16, 2017

feat(@angular/cli): support sourcemaps and minification in scripts
Adds sourcemap and minification to javascript added via the `scripts` array in `.angular-cli.json`.

`script-loader` is no longer used, which should help with CSP since it used `eval`.

Scripts will no longer appear in the console output for `ng build`, as they are now assets instead of webpack entry points.

It's no longer possible to have the `output` property of both a `scripts` and a `styles` entry pointing to the same file. This wasn't officially supported or listed in the docs, but used to be possible.

Fix #2796
Fix #7226
Fix #7290

Related to #6872

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Aug 17, 2017

feat(@angular/cli): support sourcemaps and minification in scripts
Adds sourcemap and minification to javascript added via the `scripts` array in `.angular-cli.json`.

`script-loader` is no longer used, which should help with CSP since it used `eval`.

Scripts will no longer appear in the console output for `ng build`, as they are now assets instead of webpack entry points.

It's no longer possible to have the `output` property of both a `scripts` and a `styles` entry pointing to the same file. This wasn't officially supported or listed in the docs, but used to be possible.

Fix #2796
Fix #7226
Fix #7290

Related to #6872

Brocco added a commit that referenced this issue Aug 17, 2017

feat(@angular/cli): support sourcemaps and minification in scripts
Adds sourcemap and minification to javascript added via the `scripts` array in `.angular-cli.json`.

`script-loader` is no longer used, which should help with CSP since it used `eval`.

Scripts will no longer appear in the console output for `ng build`, as they are now assets instead of webpack entry points.

It's no longer possible to have the `output` property of both a `scripts` and a `styles` entry pointing to the same file. This wasn't officially supported or listed in the docs, but used to be possible.

Fix #2796
Fix #7226
Fix #7290

Related to #6872

This comment has been minimized.

Show comment Hide comment
@moiz-h

moiz-h Aug 31, 2017

+1 for getting rid of eval() and new Function calls. We get the evalError when trying to use an Angular component in an existing AngularJS app.

moiz-h commented Aug 31, 2017

+1 for getting rid of eval() and new Function calls. We get the evalError when trying to use an Angular component in an existing AngularJS app.

This comment has been minimized.

Show comment Hide comment
@appeality

appeality Sep 7, 2017

+1 - There's a single call to eval.call(...) in scripts.bundle.js that prevents proper use of a CSP response header in Angular apps.

+1 - There's a single call to eval.call(...) in scripts.bundle.js that prevents proper use of a CSP response header in Angular apps.

@ankemp ankemp referenced this issue in alexabbott/firebase-cms Sep 8, 2017

Open

Security, Pen Testing & Scaling #10

This comment has been minimized.

Show comment Hide comment
@danielmapar

danielmapar Sep 13, 2017

Beside AOT, any temporary fix for this guys?

Beside AOT, any temporary fix for this guys?

This comment has been minimized.

Show comment Hide comment
@xnnkmd

xnnkmd Oct 23, 2017

@filipesilva Is this fixable ?

xnnkmd commented Oct 23, 2017

@filipesilva Is this fixable ?

This comment has been minimized.

Show comment Hide comment
@runes83

runes83 Nov 15, 2017

Any news on this?

runes83 commented Nov 15, 2017

Any news on this?

This comment has been minimized.

Show comment Hide comment
@filipesilva

filipesilva Dec 7, 2017

Member

@clydin can you weigh in with the results from when you tested CSP last pleast?

Member

filipesilva commented Dec 7, 2017

@clydin can you weigh in with the results from when you tested CSP last pleast?

This comment has been minimized.

Show comment Hide comment
@darrenmothersele

darrenmothersele Dec 13, 2017

I think this might be preventing me from using Angular 5 to create a WebExtension.
I get this error...
angular-csp-01

I think this might be preventing me from using Angular 5 to create a WebExtension.
I get this error...
angular-csp-01

This comment has been minimized.

Show comment Hide comment

This comment has been minimized.

Show comment Hide comment
@clydin

clydin Jan 9, 2018

Contributor

With a production build and the removal of the reflection polyfills (these are not needed if using AOT which is enabled by default for production builds), eval is only used in two locations due to the use of the global object webpack shim. This shim is planned to be disabled in the next major CLI release. However, the use is guarded in a try/catch and includes fallback code so the usage is not required for a functioning application. Unfortunately, the warnings will still appear when running static analysis tools.

Also, please note that nothing in this issue precludes the use of CSP as a whole.

Contributor

clydin commented Jan 9, 2018

With a production build and the removal of the reflection polyfills (these are not needed if using AOT which is enabled by default for production builds), eval is only used in two locations due to the use of the global object webpack shim. This shim is planned to be disabled in the next major CLI release. However, the use is guarded in a try/catch and includes fallback code so the usage is not required for a functioning application. Unfortunately, the warnings will still appear when running static analysis tools.

Also, please note that nothing in this issue precludes the use of CSP as a whole.

This comment has been minimized.

Show comment Hide comment
@sherlock1982

sherlock1982 Mar 9, 2018

Even with AOT styles created in Components are rendered as inline. Is it intended? If yes I will rewrite it of course.

Even with AOT styles created in Components are rendered as inline. Is it intended? If yes I will rewrite it of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment