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

Jump to bottom

sanitize tracking code displayed in the UI on output, not input #8123

Open
diosmosis opened this issue Jun 16, 2015 · 3 comments
Open

sanitize tracking code displayed in the UI on output, not input #8123

diosmosis opened this issue Jun 16, 2015 · 3 comments
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Technical debt Issues the will help to reduce technical debt
Milestone
Priority Backlog ...

Comments

@diosmosis
Copy link
Member

In TrackingCodeGenerator::generate(), htmlentities() is used (improperly) to escape HTML characters. The result is then outputted w/o escaping in _displayJavascriptCode.twig. Instead, TrackingCodeGenerator should return JS code w/o any additional processing/escaping, and it should be escaped only in HTML/XML output.

This is BC breaking since it affects API output. Users of that API currently will have to unsanitize or display the text w/o escaping, so it may break uses.

Refs #4231, #8109
@diosmosis diosmosis added c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels Jun 16, 2015
@diosmosis diosmosis added this to the 3.0.0 milestone Jun 16, 2015
This was referenced Jun 16, 2015
Merged
Closed
@tsteur
Copy link
Member

tsteur commented Jun 17, 2015

This is BC breaking since it affects API output. User of that API currently will have to unsanitize or display the text w/o escaping, so it may break uses.

I'm not quite sure I understand. What exactly will break? Meaning what is the output before and after? Will people still be able to fetch the tracking code from the API and insert it automatically into the website? As it is 3.0.0 it is probably less important re BC but asking as there is already one issue merged. Hope we're not breaking API before :)

@diosmosis
Copy link
Member Author

Right now, TrackingCodeGenerator will return already escaped output, which means SitesManager.getJavascriptTag will return escaped output, even if the format is JSON. After this issue is closed SitesManager.getJavascriptTag should return unescaped output for JSON results. If users are expecting escaped output, then their code may break.

There is no related BC break in 2.14.

@mattab
Copy link
Member

mattab commented Aug 13, 2015

Goals:

@mattab mattab added the Technical debt Issues the will help to reduce technical debt label Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Technical debt Issues the will help to reduce technical debt
Projects
None yet
Milestone
Priority Backlog (Help wanted)
Development

No branches or pull requests
3 participants
@diosmosis @tsteur @mattab