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

Check that printing GET parameters in the JS code is secure #5498

Closed
mattab opened this issue Jan 10, 2008 · 2 comments
Closed

Check that printing GET parameters in the JS code is secure #5498

mattab opened this issue Jan 10, 2008 · 2 comments
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Jan 10, 2008

in [source:/trunk/modules/ViewDataTable.php] method`
getJavascriptVariablesToSet()` we load GET parameters values and print them in the javascript code to “forward” the values to the Javascript logic (used in the Jquery code).

Is this safe? We use`
Piwik_Common::getRequestVar()` to sanitize the value but is it safe enough? Or could some hijacking/xss/etc be possible here?

@anonymous-matomo-user
Copy link

Just a suggestion – you probably only want to sanitize HTML tags and quotes. The actual data of the request should be left as is as much as possible, or at least kept in strings when output to JS.

That said, just about anything can get past a typical filter these days – have a brief glance through (ha.ckers.org/xss.html) this cheat sheet for XSS], it’s clearly impractical to protect data against just about anything. As long as arbitrary JS can’t go straight from the URL to the scripts (unless this is intentional, of course), there really is no cause for concern.

The htmlspecialchars() in Piwik_Common::getRequestVar() is sufficient, and maybe an addslashes() somewhere is an option.

@mattab
Copy link
Member Author

mattab commented Mar 17, 2008

(In 383) – fix #5498 Thanks for your help on this Draicone. Added addslashes() to the values printed in the JS footer of the datatables

@mattab mattab added this to the DVNO milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

2 participants