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

calendar: potential XSS when using user-supplied timeOnlyTitle translation #4385

Closed
cnsgithub opened this Issue Jan 10, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@cnsgithub
Copy link
Contributor

cnsgithub commented Jan 10, 2019

1) Environment

  • PrimeFaces version: 6.3-snapshot

2) Expected behavior

timeOnlyTitle is contained in PrimeFaces.locales (https://github.com/primefaces/primefaces/wiki/Locales) and therefore must be considered as user-supplied input.

3) Actual behavior

The value is passed through to jquery.ui.timepicker.js without being escaped there:

$tp.prepend('<div class="ui-widget-header ui-helper-clearfix ui-corner-all">' + '<div class="ui-datepicker-title">' + o.timeOnlyTitle + '</div>' + '</div>');

This is actually a bug in jquery.ui.timepicker.js. However, since PrimeFaces bundles it, it should be fixed here as well.

Basically all strings contained in PrimeFaces.locales should be paid extra attention.

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Jan 10, 2019

I think we pass "o" to the timepicker somehow?
It would be great if we could do something like this in PF, instead of changes the timepicker.js:
o.timeOnlyTitle = escape(o.timeOnlyTitle);

@cnsgithub

This comment has been minimized.

Copy link
Contributor Author

cnsgithub commented Jan 10, 2019

That's kind of problematic, since we cannot know where o.timeOnlyTitle is used (in fact we currently know, but things might change 😉 ) and therefore don't know the context it's used in. So we should not assume it's being used in HTML context only.

There is no link to the lib provided in jquery.ui.timepicker.js that might be used to check for newer versions?

@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Jan 10, 2019

@cnsgithub the TimePicker jquery addon is found here: https://github.com/trentrichardson/jQuery-Timepicker-Addon

@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Jan 10, 2019

It looks like the original Author Trent Richardson is no longer maintaining it. trentrichardson/jQuery-Timepicker-Addon#955 We usually make fixes to this component using the pfextensions.js file but since its abandoned we can probably just start editing this add on directly?

@cnsgithub

This comment has been minimized.

Copy link
Contributor Author

cnsgithub commented Jan 11, 2019

@melloware I agree.

@cnsgithub

This comment has been minimized.

Copy link
Contributor Author

cnsgithub commented Jan 14, 2019

PR: #4426

mertsincan added a commit that referenced this issue Jan 16, 2019

Merge pull request #4426 from cnsgithub/fixes-4385-xss-calendar-timeo…
…nlytitle

fix #4385 - calendar: potential XSS when using user-supplied timeOnlyTitle translation

@mertsincan mertsincan added this to the 7.0 milestone Jan 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment