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

Do not use eval function in piwik.js (for CSP) #5960

Closed
ondrejmirtes opened this issue Aug 9, 2014 · 13 comments
Closed

Do not use eval function in piwik.js (for CSP) #5960

ondrejmirtes opened this issue Aug 9, 2014 · 13 comments
Assignees
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@ondrejmirtes
Copy link

In order to implement CSP on my website, I disallowed 'unsafe-eval' in script-src directive. But I cannot track visitors with Piwik, because piwik.js tracking code contains call to eval function twice.

Can you consider rewriting the code so it does not use eval? (Which is a bad idea nevertheless.)

@mattab
Copy link
Member

mattab commented Aug 20, 2014

One of the two eval is used in the JSON2 implementation. We can't change that unfortunately.

@mattab mattab added the wontfix label Aug 20, 2014
@mattab mattab closed this as completed Aug 21, 2014
@ondrejmirtes
Copy link
Author

What is the included JSON2 library for, since all current browsers support JSON.parse and JSON.stringify?

@mattab
Copy link
Member

mattab commented Aug 25, 2014

@ondrejmirtes it's for all "non-current" browsers that are still tracked by Piwik.

@ondrejmirtes
Copy link
Author

So what about eliminating the other (second) eval call that's not part of the JSON2 library and using native JSON.parse/stringify where available? That way, it would be possible to remove 'unsafe-eval' clausule from my CSP string and still track overwhelming majority of users.

I personally don't care about supporting ancient browsers (the whole caniuse overview table is green) and would rather prefer a more secure experience for the majority of users. I suspect most of the developers interested in using Piwik platform would take the same stance.

@mattab
Copy link
Member

mattab commented Aug 25, 2014

I personally don't care about supporting ancient browsers

Most people care about tracking as many visits as possible, so I'm afraid it's not as simple as it sounds. Maybe you can create a new issue for this specific change?

@ondrejmirtes
Copy link
Author

Well, people who don't setup CSP header would still be able to track them, it's only that in my server configuration, the minority of browsers that would trigger the eval call would not get tracked.

And I realized that the browsers that support CSP also probably have native JSON implementation, so the number of untracked visitors would be zero.

@ondrejmirtes
Copy link
Author

My whole motivation behind this issue is to encourage Piwik to stay on the cutting edge of web standards and security because I really like it and want to promote it in the community. So please think about some solutions to enable using strict CSP settings while tracking with Piwik.

@mattab
Copy link
Member

mattab commented Aug 25, 2014

@ondrejmirtes sounds good, thanks! Could you maybe create a new issue with more specific scope than "do not use eval"?

For example: Make Piwik compatible with strict CSP settings ?

@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Sep 20, 2014
@tsteur
Copy link
Member

tsteur commented Feb 17, 2015

I wonder why this issue is closed as it is not fixed? For example we could use https://bestiejs.github.io/json3/ which does not use eval. I just decided to reopen it :)

@tsteur tsteur reopened this Feb 17, 2015
@tsteur tsteur removed the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Feb 17, 2015
@tsteur
Copy link
Member

tsteur commented Feb 17, 2015

FYI: I tried to replace JSON2 with JSON3 but I can't get JSON3 to work with JSLint . Meaning no matter which options I set JSLint always fails and it seems to be not possible to ignore one block of code for JSLint. We should replace JSLint at some point with JSHint which would exactly solve this.

@tsteur tsteur added this to the Short term milestone Feb 17, 2015
@tsteur
Copy link
Member

tsteur commented Feb 17, 2015

If someone manages to make Piwik.js work with JSHint we could replace JSON2 with JSON3 and remove all evals. See #7232

@mattab mattab changed the title Do not use eval function in JavaScript Do not use eval function in piwik.js (for CSP) Apr 9, 2015
@mattab mattab added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. and removed Bug For errors / faults / flaws / inconsistencies etc. labels Apr 9, 2015
@mattab
Copy link
Member

mattab commented Apr 9, 2015

since the goal of this issue is really to add CSP support to piwik.js I suggest we mark as duplicate of: #1542

next step: #7232

@mattab mattab closed this as completed Apr 9, 2015
@mattab mattab added the duplicate For issues that already existed in our issue tracker and were reported previously. label Apr 9, 2015
@tsteur
Copy link
Member

tsteur commented Oct 2, 2015

Reopen this one since eval !== CSP

@tsteur tsteur reopened this Oct 2, 2015
@tsteur tsteur modified the milestones: 2.15.0, Short term Oct 2, 2015
@tsteur tsteur added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. and removed duplicate For issues that already existed in our issue tracker and were reported previously. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. labels Oct 2, 2015
@tsteur tsteur self-assigned this Oct 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

3 participants