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

Inline Event Handlers Triggering Content Security Policy Warnings #403

Closed
cbellgit opened this issue Dec 7, 2017 · 6 comments
Closed

Inline Event Handlers Triggering Content Security Policy Warnings #403

cbellgit opened this issue Dec 7, 2017 · 6 comments

Comments

@cbellgit
Copy link

cbellgit commented Dec 7, 2017

The inline event handlers used for the action buttons have inline event handlers to trigger functionality.
For example:
<div title="Edit selected row" class="ui-corner-all ui-pg-div ui-inline-edit" id="jEditButton_1" onclick="return jQuery.fn.fmatter.rowactions.call(this,event,'formedit');" onmouseover="jQuery(this).addClass('ui-state-hover');" onmouseout="jQuery(this).removeClass('ui-state-hover');"><span class="ui-icon ui-icon-pencil"></span></div>
The onclick, onmouseover and onmouseout events don't play well with content security policy. What would be involved in changing the code to use event listeners (.addEventListener('click', function () {})) instead? Is there a performance reason for leaving them as they are or is it just the way it's always been?

@OlegKi
Copy link
Member

OlegKi commented Dec 7, 2017

Which version of free jqGrid you use? Which web browser in which version you used? Which Content Security Policies (CSP) exactly you use on your page (you can specify specific settings for every HTML page) and so on? Do you specify the policies via HTTP headers or via <meta http-equiv="Content-Security-Policy" content="..."? Could you provide the demo, which reproduces the problem? In general, for example, the demo https://jsfiddle.net/OlegKi/yvbt6w54/ contains formatter: "actions" and it works.

@cbellgit
Copy link
Author

cbellgit commented Dec 8, 2017

I am using version free jqGrid 4.15.2 in Chrome Version 62.0.3202.94. Also using firefox 57.0.1. The CSP I am currently using (via headers) is
Content-Security-Policy-Report-Only:default-src 'self'; script-src 'self' 'nonce-16charrandomnonce'; style-src 'self' 'unsafe-inline'; report-uri /error/cspviolation

where 16charrandomnonce is replaced with a random base64 encoded number that is regenerated every page load. I am currently using 'Report-Only' as I begin to implement the CSP during testing.

Every time I mouse over or mouse out an action icon chrome, gives the message
[Report Only] Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'self' 'nonce-16charrandomnonce'". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution.

Firefox gives the message
Content Security Policy: The page’s settings observed the loading of a resource at self (“script-src https://server.com 'nonce-16charrandomnonce'”). A CSP report is being sent. Source: onmouseover attribute on DIV element.

I wasn't able to quickly get CSP working in jsfiddle. I moved your demo to my server and added a CSP meta tag to it: https://www.inolleb.com/jqgdemo/

If you load that page, then use the developer tools to view the console in either chrome or firefox, then hover over the action icons (or more importantly, click on them) you should see warnings similar to what I have indicated and the clicks should be blocked,

Of course, I could add 'unsafe-inline' to the script-src directive of the CSP, but that would pretty much defeat the purpose of the CSP.

@OlegKi
Copy link
Member

OlegKi commented Dec 8, 2017

Thank you for the demo and detailed description of the problem. I'm now still in the business trip, but I'll try to make all required changes in the free jqGrid at the weekend. I'll post you short message after I'll publish the changes to GitHub.

OlegKi added a commit that referenced this issue Dec 10, 2017
…r and to allow to use jqGrid with less restrictive Content Security Policy

Thanks @cbellgit (https://github.com/cbellgit) for pointing on the problem. See [the issue #403](#403) for more details.
@OlegKi
Copy link
Member

OlegKi commented Dec 10, 2017

I posted some changes to the code of free jqGrid to fix the problem of unsafe-inline setting of the CSP. Your demo uses the code from GitHub and to allows to use formatter: "actions" now.

I want to mention, that the current code of free jqGrid has still some other places which can be improved too. For example, data grouping feature has the close problem with the usage of inline event handle like formatter: "actions". I'll make the corresponding changes later (probably at the next weekend :-)). Local filtering/searching feature has the problem with unsafe-eval setting of the CSP. It's more complex to fix, but I'll examine the problem later and will consider to make the corresponding changes. Till the time one will have to add script-src 'unsafe-eval' in the Content Security Policy settings of the page.

@cbellgit
Copy link
Author

That did the trick. Thank you for your prompt response to this and for your ongoing commitment to this project.

OlegKi added a commit that referenced this issue Dec 17, 2017
The changes continues the changes from [the commit](13c3931). See [the issue #403](#403) for more details.

Now one should be able to use `Content-Security-Policy` with `script-src 'self'`. The problem with the usage of `eval` during local searching/filtering still exist. Thus one have to use `script-src 'self' 'unsafe-eval' ...; style-src 'unsafe-inline' ...`
@OlegKi
Copy link
Member

OlegKi commented Dec 17, 2017

I committed more changes of the code of free jqGrid (see here), which are related the code of data grouping. Now the full code of jqGrid should not more contains any inline handlers and the usage of script-src 'self' should be possible. The problem with inline CSS styles and 'unsafe-eval' (for local searching/filtering of data) till stay. It's more complex and I don't plan to fix the problem in the next release of free jqGrid (probably sometime later).

I'm closing the issue now because I think that the reported problem is solved.

@OlegKi OlegKi closed this as completed Dec 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants