Navigation Menu

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

xss affecting IE6, IE7 and IE8 #6053

Closed
mattab opened this issue Aug 23, 2014 · 7 comments
Closed

xss affecting IE6, IE7 and IE8 #6053

mattab opened this issue Aug 23, 2014 · 7 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. 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.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Aug 23, 2014

Here is the report to our security team

reproduce

piwik_xss_scrennshot

details

If you check the source code, it is basically caused by the following code in API.php: https://github.com/piwik/piwik/blob/8d1f1b39f26bc00394af5ac220b6bd97ca89537f/plugins/SitesManager/API.php#L129

There is no check or validation for the parameter piwikUrl to filter malicious characters. This vulnerability has been there since Piwik 2.2.0 Version. Please let me know. In the attachment is a screenshot of the exploitation against demo.piwik.org website in IE7.

Note from Piwik security team

We generally do not communicate via issues on XSS issues in Piwik (some have called it security by obscurity but we definitely disagree with that). Because this issue affects only IE6 IE7 IE8 it has limited impact and so we decided to track it in a github issue.

@mattab mattab added this to the Piwik 2.6.0 milestone Aug 23, 2014
@mattab
Copy link
Member Author

mattab commented Aug 23, 2014

Once this issue is verified and reproduced, we shall send a bounty to the security researcher Dingjie (Daniel) Yang, as per our security bug bounty program.

@kylekatarnls
Copy link
Contributor

Something like that included in index.php could fix it:

$getParamsWithHtmlAllowed = array('foo', 'bar');
foreach(array_diff_key($_GET, array_flip($getParamsWithHtmlAllowed)) as $key => $value) {
    $_GET[$key] = htmlspecialchars($value);
}

No?

@ThaDafinser
Copy link
Contributor

@kylekatarnls a "rule" for all is also dangerous...please not this way 😄

@kylekatarnls
Copy link
Contributor

By default, any parameter need html, so if any exceptions are forgotten, no dangers. And a global is more efficient than dupplicate a code in several places. So what do you suggest? Give it if you have a better way.

@mattab
Copy link
Member Author

mattab commented Aug 27, 2014

@kylekatarnls actually @ThaDafinser is right, the global is not a proper solution, as it would break many other things. We'll find proper fix no worries.

@ThaDafinser
Copy link
Contributor

@kylekatarnls remember magic_quotes? 😄 that was the same approach....i think i dont have to say anything more ;-)
http://php.net/manual/de/security.magicquotes.php

@mattab mattab modified the milestones: Piwik 2.8.0, Piwik 2.7.0 Sep 7, 2014
@mattab
Copy link
Member Author

mattab commented Sep 7, 2014

This security issue occurs because the url contains /index.php/.html?

The solution we are discussing is to redirect URL so index.php/..../?... urls are turned into index.php?...

@mattab mattab modified the milestones: Piwik 2.9.0, Piwik 2.8.0 Oct 7, 2014
@mnapoli mnapoli self-assigned this Oct 10, 2014
@mattab mattab modified the milestones: Piwik 2.9.0, Piwik 2.8.1 Oct 19, 2014
@mnapoli mnapoli closed this as completed Oct 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. 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.
Projects
None yet
Development

No branches or pull requests

4 participants