Skip to content
Permalink
Browse files Browse the repository at this point in the history
RA-1772: Fix stored xss is appointment scheduling (#32)
  • Loading branch information
isears committed Jun 9, 2020
1 parent 30f9be0 commit 34213c3
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
Expand Up @@ -18,6 +18,7 @@
import org.openmrs.annotation.Handler;
import org.openmrs.module.appointmentscheduling.AppointmentType;
import org.openmrs.module.appointmentscheduling.api.AppointmentService;
import org.openmrs.web.WebUtil;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.validation.Errors;
Expand Down Expand Up @@ -82,6 +83,9 @@ private void validateFieldName(Errors errors, AppointmentType appointmentType) {
if (verifyIfNameHasMoreThan100Characters(appointmentType.getName())) {
errors.rejectValue("name", "appointmentscheduling.AppointmentType.longName.errorMessage");
}
if(verifyIfNameHasHtmlEncodableChars(appointmentType.getName())){
errors.rejectValue("name", "appointmentscheduling.AppointmentType.unsafeName.errorMessage");
}
}

private boolean verifyIfNameHasMoreThan100Characters(String appointmentName) {
Expand Down Expand Up @@ -110,5 +114,12 @@ private boolean verifyIfDescriptionHasMoreThan1024Characters(String description)
}
return false;
}

private boolean verifyIfNameHasHtmlEncodableChars(String appointmentName) {
if(appointmentName != null){
return !WebUtil.escapeHTML(appointmentName).equals(appointmentName);
}
return false;
}

}
1 change: 1 addition & 0 deletions api/src/main/resources/messages.properties
Expand Up @@ -24,6 +24,7 @@ ${project.parent.artifactId}.AppointmentType.duration.errorMessage = Duration mu
${project.parent.artifactId}.AppointmentType.steps.details=Appointment Type Details
${project.parent.artifactId}.AppointmentType.nameDuplicated=Name is duplicated
${project.parent.artifactId}.AppointmentType.longName.errorMessage= Name must have less than 100 characters
${project.parent.artifactId}.AppointmentType.unsafeName.errorMessage= Name must not have HTML characters
${project.parent.artifactId}.AppointmentType.description.errorMessage = Description must have less than 1024 characters

${project.parent.artifactId}.AppointmentBlock.manage.title=Provider Scheduling
Expand Down
Expand Up @@ -94,7 +94,17 @@ public void mustRejectAppointmentTypeNameWithMoreThan100Characters() throws Exce

Mockito.verify(errors).rejectValue("name", "appointmentscheduling.AppointmentType.longName.errorMessage");
}


@Test
public void mustRejectAppointmentTypeNameWithXSS() throws Exception {
String evilName = "<script>alert(1)</script>";
AppointmentType appointmentTypeEvilName = new AppointmentType(evilName, "", 10);

appointmentTypeValidator.validate(appointmentTypeEvilName, errors);

Mockito.verify(errors).rejectValue("name", "appointmentscheduling.AppointmentType.unsafeName.errorMessage");
}

@Test
public void mustRejectAppointmentTypeDescriptionWithMoreThan1024Characters() throws Exception {
String longDescription = StringUtils.repeat("*", 1025);
Expand Down

0 comments on commit 34213c3

Please sign in to comment.