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 vulnerability in DateTimePicker Component due to lack of xml escaping #1133

Closed
rgdoms opened this issue Apr 8, 2020 · 2 comments
Closed
Assignees
Labels
confirmed security Solved This ticket has been solved. If it's still open, we're waiting for the user's confirmation.
Milestone

Comments

@rgdoms
Copy link

rgdoms commented Apr 8, 2020

During some security tests, we concluded that although DateTimePicker escapes XML content in value field, the same is used in the generated script, in date field, without escaping. This allows cross site scripting and unwanted code may be run by malicious actions.

Checkout the following examples. Here we forced the value (by editing the Html) with a js script.
In the renderer generated code, markup code is escaped here:
<input type="text" id="myform:myperiod_Input" name="myform:myperiod" class="form-control bf-required" placeholder="aaaa - mm" value="2020 - 02--&gt;&lt;/sCrIpT&gt;&lt;sCrIpT&gt;alert(51845)&lt;/sCrIpT&gt;" />

And here, it is not.
</script><script>$(function () { $('#myform\\:myPeriodOuter').datetimepicker({ ignoreReadonly: false, allowInputToggle: true, collapse: true, dayViewHeaderFormat: 'MMMM YYYY', focusOnShow: true, minDate: '2020-01', maxDate: '2020-03-30', stepping: 1, toolbarPlacement: 'default', viewMode: 'days', useCurrent:false,date: moment('2020 - 02--></sCrIpT><sCrIpT>alert(51845)</sCrIpT>', 'YYYY - MM'), locale: 'pt', format: 'YYYY - MM'});});</script>

This is due to the following:
In DateTimePickerRenderer the value is written via writeAttribute method of ResponseWriter class, that ensures that markup code is escaped.

if (v != null) {
rw.writeAttribute("value", getValueAsString(v, fc, dtp), null);

In the generated script, the renderer gets the value and no escaping is performed. getValueAsString calls getDateAsString, and this calls getInternalDateAsString that returns a formated date, or getAsString from a custom converter (which is our case).

String inlineDisplayDate = "'" +
getValueAsString(v, fc, dtp) + "'";

"date: moment(" + inlineDisplayDate + ", " + displayFormat + "), " +

Thanks

@stephanrauh
Copy link
Collaborator

That doesn't sound good. I can't promise anything, but I intend to have a close look at this bug this week-end. Thanks for pointing this out and for your thorough analysis!

@geopossachs geopossachs added this to the v1.5.0 milestone May 1, 2020
@stephanrauh stephanrauh added confirmed Solved This ticket has been solved. If it's still open, we're waiting for the user's confirmation. labels May 5, 2020
@stephanrauh
Copy link
Collaborator

I've just fixe. the bug by adding the OWASP HTML sanitizer. I've published a new 1.5.0-SNAPSHOT on Maven Central. Can you check if this fixes the vulnerability?

Thanks in advance,
Stephan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed security Solved This ticket has been solved. If it's still open, we're waiting for the user's confirmation.
Projects
None yet
Development

No branches or pull requests

3 participants