Impossible to assign a location with an apostrophe in the name to a user.

luis.oliveira · September 24, 2020, 2:21pm

Hi Everyone,

We found that is not possible to assign a location with an apostrophe to a user in Advance administration → User Management.

After analyses I found out that the problem is here:

github.com

openmrs/openmrs-module-datafilter/blob/5004799b582c29c6b610724f83bee34b350d97e9/omod/src/main/java/org/openmrs/module/datafilter/extension/html/Content.java#L49-L62


      
          	private String addHTMLForLocationNames() {
          		return locationNames.stream().reduce("", (appendedContent, locationName) -> {
          			String checkedValue = "";
          			appendedContent += "<span class='listItem'>";
          			if (selectedLocations.contains(locationName)) {
          				checkedValue = CHECKED;
          			}
          			appendedContent += "<input type='checkbox' name='locationStrings' id='locationStrings." + locationName + "'"
          			        + " value='" + locationName + "'" + checkedValue + ">";
          			appendedContent += "<label for='locationStrings." + locationName + "'>" + locationName + "</label>";
          			appendedContent += "</span>";
          			return appendedContent;
          		});
          	}

When the HTML code is being generated, the variable locationName is being surrounded by ' . Meaning that, when the HTML is being interpreted, if the locationName as a ' it will be considered the end of the text.

A possible solution is to replace the ' in the code by a ".

Please let me know what do you think.

cc: @mksd

tansentim · September 24, 2020, 7:57pm

Hi All,

I think that this would cause the same problem if someone used “ in a name. Is not the proper way to ensure the locationName is appropriately escaped for HTML? '.

But also all html characters such as < > & etc should also be escaped.

tansentim · September 24, 2020, 8:16pm

That symbol should be & a p o s ; but as one string '

luis.oliveira · September 25, 2020, 7:55am

Nice catch, is not expected that situations for locations names, but is better protect for all the cases.

luis.oliveira · October 1, 2020, 10:24am

Please follow the issue here.