Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Axe-core fails under strict content security policy due to eval script (EvalError) #1175

Closed
drewlee opened this issue Oct 3, 2018 · 15 comments
Assignees
Labels
core Issues in the core code (lib/core) fix Bug fixes

Comments

@drewlee
Copy link

drewlee commented Oct 3, 2018

Axe-core script fails under strict content security policy which disallows unsafe eval() executions. The source of the violations is stemming from the doT library and is documented here: olado/doT#276.

The issues can be attributed to two specific areas:

axe.imports['doT'] = function(module, exports, define, require, process) {
    var global = Function('return this')();
    ...
    ...
     _globals = function() {
        return this || (0, eval)('this');
      }();
    ...

Mitigating these two areas of the script seems to resolve the issue as demoed under the fixed link below. It seems that this package (doT) is no longer under active development, and the owners do not care for addressing the security violations. It would be advisable to avoid this package altogether, if possible.

axe-core version: 3.1.2
@WilcoFiers
Copy link
Contributor

Thanks for reporting, and for digging into this. That's helpful. @stephenmathieson is this code we added or does this come from doT? Any thoughts on a workaround?

@stephenmathieson
Copy link
Member

@WilcoFiers the short answer is "both". The doT template engine uses both eval and the Function constructor several times throughout its codebase.

Last month, I tried to refactor the linked LOCs, but due to all of the evals in doT, I don't think there's anything we can do here.

@drewlee
Copy link
Author

drewlee commented Oct 4, 2018

Thanks @WilcoFiers and @stephenmathieson for the details. This issue is currently blocking us from integrating axe-core into our testing story. We don't have the option to change our SCP. I don't know to what extent axe-core requires a templating engine, but doT doesn't look like it's under active development and is stale. Has there been any thought for replacement? I'm sure there are comparable solutions from a perf perspective. Forking doT might be another option. Would be happy to provide assistance. Thanks!

@lhorie
Copy link

lhorie commented Oct 5, 2018

FYI this is a dupe of dequelabs/react-axe#54 and #1158

doT is not the only place where eval-likes are being used. I found a usage of new Function in audit.js, as per the original issue.

To solve this issue, you need to refactor to not use dynamic code generation (e.g. replace things like new Function('return this') with window

The code in audit.js seems tricky to modify and I don't have enough context to be able to give a recommendation. Ideally, you should refactor the code so that metadata.messages[prop] is a function to begin with (i.e. by passing the function in, rather than stringified source code). Another option would be to drop that snippet altogether if it's code to handle a nice-to-have API overload, then deprecate said feature. Another relatively more complex option is do what Angular.js does (implement an interpreter). If the number of possible functions is small, another option would be to write them all out and select the appropriate one via a lookup table.

@drewlee
Copy link
Author

drewlee commented Oct 5, 2018

Yeah, some of those eval Function calls are a means of getting a reference to the global object in support of interoperability between Node and browser JS. Referencing window directly wouldn't be the most appropriate approach, but there are definitely less contentious (and more readable) means of referencing the global object.

@muan
Copy link

muan commented Feb 27, 2019

Just to add a data point, we are also blocked from upgrading past ~3.0 by this, and looks like this is likely going to be the case going forward? I see that #1024 has noted that moving away from doT would require significant effort.

kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Jun 6, 2019
The Axe-core library that govuk_publishing_components uses makes use of
a dependency that uses the eval function so this directive is needed
until that is removed.

Relevant project issues:

- dequelabs/axe-core#1175
- olado/doT#276
kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Jun 10, 2019
The Axe-core library that govuk_publishing_components uses makes use of
a dependency that uses the eval function so this directive is needed
until that is removed.

Relevant project issues:

- dequelabs/axe-core#1175
- olado/doT#276

This effects of this issue is that all use of component guides trigger
CSP errors for every page view. Oddly this issue doesn't seem to be
observable in Chrome when report only is on, but can be seen in Firefox.
@dylanb
Copy link
Contributor

dylanb commented Jun 16, 2019

Our efforts to support the Chrome Manifest V3 may provide a solution to this but that would be a breaking change and would require axe-core v4. Custom rules would break too - which would impact @muan

@straker
Copy link
Contributor

straker commented Jul 17, 2019

The best we can do for now is to remove all evals and Function('') calls in the critical path. Any use of axe-core with custom translations and/or rules will still have the eval problem until we can remove them in v4.

@jeankaplansky
Copy link
Contributor

No product docs required.

@somaalapati
Copy link

@straker Is this testable by QA? I think we need a test site that has SCP. Need you inputs on it. Please close this issue if it is not testable by QA.

@straker
Copy link
Contributor

straker commented Aug 2, 2019

QA can verify using the two CodePens:

shows CSP error: https://codepen.io/drewlee/full/qJZZbx
shows no CSP error (fix): https://codepen.io/straker/pen/GVKwde

@somaalapati
Copy link

somaalapati commented Aug 5, 2019

@chandana7393 Please test this. Let me know if you need help.
Also, add this as a test scenario in our test case base I mean Jira.

@chandana7393
Copy link

Tested, working as expected.
no_issue_console
Tested Environment:
Attest - 2.5.1.21421v
Axe - 3.8.1.21421v
Axe-coconut - 3.8.1.21704v
Chrome - 76.0.3809.87v
Firefox - 68.0.1v
OS - Windows 10 64 bit.

@drewlee
Copy link
Author

drewlee commented Aug 7, 2019

@chandana7393 the pen you used for verification doesn't actually include the axe-core version that has the CSP fix patch. That was just simply a demo of a suggested fix, using v3.1.2. However, I've put up a new pen which includes v3.3.1 and have verified that it resolves the issue. See https://codepen.io/drewlee/full/zgWONO. Thanks everyone for addressing this!

kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Sep 27, 2019
This adds the GOV.UK security policy to the dummy application of this
gem. This means that when using this app in dev or viewing it at
https://components.publishing.service.gov.uk/component-guide the GOV.UK
CSP will apply.

This also has to unfortunately allow unsafe-eval for JS so that the
component-guide browser can use AXE. Relevant project issues:

- dequelabs/axe-core#1175
- olado/doT#276
@malveujvala
Copy link

Hi... I am also facing the same issue with the Axe-core integration with Java. Currently, I am using the below dependency in POM.xml file

<dependency>
		<groupId>com.deque</groupId>
		<artifactId>axe-selenium</artifactId>
		<version>3.0</version>
	</dependency>

Any plan to fix this issue with Java integration as well? I can't update the dependency to 3.5. Any workaround which I can use to fix the issue. Please let me know. I am currently blocked due to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues in the core code (lib/core) fix Bug fixes
Projects
None yet
Development

No branches or pull requests