-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fix inconsistent sanitization: sanitize on output rather than input #4231
Comments
Besides Website names, what are the other entities which are stored sanitized? I expect maybe that many entities will store as sanitize. Fixing this has some risks for XSS we have to careful. |
|
@matt If makeXssContent is removed, how do we test for XSS? |
I didn't mean to remove the xss test itself but rather I was noting that there is also an inconsistency in the way APIs inputs are (or not) sanitized. |
Note: there is another inconsistent sanitization policy in Piwik, see below function:
and: |
#6714 has been merged into this issue |
@mattab hi, as this problem is open for a long time and commonly suffered, when do you plan to fix it? |
Due to the complexity of this task and the high risk of introducing security regressions (in particular, XSS), so far we have not been able to work / schedule this project. I would suspect it will take a long time before we can tackle this fully, and likely it will be done step by step over time, plugin by plugin etc. Any contribution will be very welcome |
The way in which Piwik deals w/ sanitizing user data is inconsistent: it is sometimes done on input (data is stored sanitized), sometimes on output (less frequently). For example, website names are stored in the DB sanitized and need to be unsanitized or outputted raw in HTML. Goal names are not sanitized in the DB and need to be escaped when outputting in HTML.
This should be made consistent. Additionally, security best practices recommend:
Problems with the current approach:
We should try to slowly move to sanitizing on output using native escaping features of Twig or AngularJS for example.
The text was updated successfully, but these errors were encountered: