diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/GenericUiServlet.java b/app/src/main/java/ch/sbb/polarion/extension/generic/GenericUiServlet.java index edbd4d4..a808f8e 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/GenericUiServlet.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/GenericUiServlet.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic; +import com.polarion.alm.shared.util.Pair; import com.polarion.core.util.logging.Logger; import org.apache.commons.io.IOUtils; import org.jetbrains.annotations.NotNull; @@ -16,11 +17,22 @@ import java.io.Serial; import java.net.URI; import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; public abstract class GenericUiServlet extends HttpServlet { + private static final List> ALLOWED_FILE_TYPES = Arrays.asList( + Pair.of(".js", "text/javascript"), + Pair.of(".html", "text/html"), + Pair.of(".css", "text/css"), + Pair.of(".png", "image/png"), + Pair.of(".svg", "image/svg+xml"), + Pair.of(".gif", "image/gif") + ); + private static final Logger logger = Logger.getLogger(GenericUiServlet.class); @Serial @@ -34,36 +46,39 @@ protected GenericUiServlet(String webAppName) { @VisibleForTesting static void setContentType(@NotNull String uri, @NotNull HttpServletResponse response) { - if (uri.endsWith(".js")) { - response.setContentType("text/javascript"); - } else if (uri.endsWith(".html")) { - response.setContentType("text/html"); - } else if (uri.endsWith(".png")) { - response.setContentType("image/png"); - } else if (uri.endsWith(".css")) { - response.setContentType("text/css"); - } + response.setContentType(ALLOWED_FILE_TYPES.stream().filter(f -> uri.endsWith(f.left())).map(Pair::right).findFirst() + .orElseThrow(() -> new IllegalArgumentException("Unsupported file type"))); } @Override protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { - String uri = request.getRequestURI(); - String relativeUri = uri.substring("/polarion/".length()); + String resourceUri = getInternalResourcePath(request.getRequestURI()); try { - if (relativeUri.startsWith(webAppName + "/ui/generic/")) { - serveGenericResource(response, relativeUri.substring((webAppName + "/ui/generic/").length())); - } else if (relativeUri.startsWith(webAppName + "/ui/")) { - serveResource(response, relativeUri.substring((webAppName + "/ui").length())); + if (resourceUri.startsWith("generic/")) { + serveGenericResource(response, resourceUri.substring("generic/".length())); + } else { + serveResource(response, "/" + resourceUri); } } catch (IOException e) { - logger.error("Cannot copy resource '" + relativeUri + "': " + e.getMessage(), e); + logger.error("Cannot copy resource '" + resourceUri + "': " + e.getMessage(), e); throw e; } catch (Exception e) { - logger.error("Unexpected error by getting resource '" + relativeUri + "': " + e.getMessage(), e); + logger.error("Unexpected error by getting resource '" + resourceUri + "': " + e.getMessage(), e); throw new ServletException(e.getMessage(), e); } } + private String getInternalResourcePath(String fullUri) { + String acceptablePath = "/polarion/" + webAppName + "/ui/"; + if (!fullUri.startsWith(acceptablePath)) { + throw new IllegalArgumentException("Unsupported resource path"); + } + if (ALLOWED_FILE_TYPES.stream().noneMatch(t -> fullUri.endsWith(t.left()))) { + throw new IllegalArgumentException("Unsupported file type"); + } + return fullUri.substring(acceptablePath.length()); + } + @VisibleForTesting void serveGenericResource(@NotNull HttpServletResponse response, @NotNull String uri) throws IOException, URISyntaxException { final URI currentJar = GenericUiServlet.class.getProtectionDomain().getCodeSource().getLocation().toURI(); diff --git a/app/src/main/resources/js/custom-select.js b/app/src/main/resources/js/custom-select.js index 18f44c7..4d8c4f0 100644 --- a/app/src/main/resources/js/custom-select.js +++ b/app/src/main/resources/js/custom-select.js @@ -128,7 +128,8 @@ SbbCustomSelect.prototype.selectMultipleValues = function(values) { SbbCustomSelect.prototype.handleChange = function(event) { if (this.mutiselect) { - this.selectElement.innerHTML = ""; + // using the code without wrapping document.createTextNode().textContent causes XSS vulnerability + this.selectElement.innerHTML = ""; if (this.changeListener) { this.changeListener(this.checkboxContainer.querySelectorAll('input[type="checkbox"]:checked')); } @@ -146,7 +147,8 @@ SbbCustomSelect.prototype.handleChange = function(event) { } }); - this.selectElement.innerHTML = ""; + // using the code without wrapping document.createTextNode().textContent causes XSS vulnerability + this.selectElement.innerHTML = ""; this.checkboxContainer.querySelectorAll('label').forEach(function (label) { label.classList.remove("selected"); if (label.textContent === selectedCheckbox.parentElement.textContent) { diff --git a/app/src/test/java/ch/sbb/polarion/extension/generic/GenericUiServletTest.java b/app/src/test/java/ch/sbb/polarion/extension/generic/GenericUiServletTest.java index 295a1fa..167f322 100644 --- a/app/src/test/java/ch/sbb/polarion/extension/generic/GenericUiServletTest.java +++ b/app/src/test/java/ch/sbb/polarion/extension/generic/GenericUiServletTest.java @@ -9,6 +9,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.Serial; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; @@ -25,37 +26,45 @@ void testSetContentType() { GenericUiServlet.setContentType("/sub_path/file.html", response); verify(response, times(1)).setContentType("text/html"); + response = mock(HttpServletResponse.class); + GenericUiServlet.setContentType("https://localhost/styles.css", response); + verify(response, times(1)).setContentType("text/css"); + response = mock(HttpServletResponse.class); GenericUiServlet.setContentType("/img.png", response); verify(response, times(1)).setContentType("image/png"); response = mock(HttpServletResponse.class); - GenericUiServlet.setContentType("https://localhost/styles.css", response); - verify(response, times(1)).setContentType("text/css"); + GenericUiServlet.setContentType("/img.svg", response); + verify(response, times(1)).setContentType("image/svg+xml"); response = mock(HttpServletResponse.class); - GenericUiServlet.setContentType("unknown_file.xml", response); - verify(response, times(0)).setContentType(any()); + GenericUiServlet.setContentType("/img.gif", response); + verify(response, times(1)).setContentType("image/gif"); + + HttpServletResponse servletResponse = mock(HttpServletResponse.class); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> GenericUiServlet.setContentType("unknown_file.xml", servletResponse)); + assertEquals("Unsupported file type", exception.getMessage()); } @Test @SneakyThrows void testService() { - // unreal case (at least we assume that uri will start with /polarion/) - assertThrows(StringIndexOutOfBoundsException.class, () -> callServlet("/badUrl")); + // error case (at least we assume that uri will start with /polarion/) + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> callServlet("/badUrl/someImg.png")); + assertEquals("Unsupported resource path", exception.getMessage()); - // does nothing coz we do not process sub-paths other than /ui/generic/ and /ui/ - TestServlet servlet = callServlet("/polarion/testServletName/unknownPath"); - verify(servlet, times(0)).serveGenericResource(any(), any()); - verify(servlet, times(0)).serveResource(any(), any()); + // we expect uri starting by /polarion/{web_app_name}/ui/ + exception = assertThrows(IllegalArgumentException.class, () -> callServlet("/polarion/testServletName/unknownPath")); + assertEquals("Unsupported resource path", exception.getMessage()); // generic resource - servlet = callServlet("/polarion/testServletName/ui/generic/genericUri"); - verify(servlet, times(1)).serveGenericResource(any(), eq("genericUri")); + TestServlet servlet = callServlet("/polarion/testServletName/ui/generic/genericUri/someImage.gif"); + verify(servlet, times(1)).serveGenericResource(any(), eq("genericUri/someImage.gif")); verify(servlet, times(0)).serveResource(any(), any()); // regular resource - servlet = callServlet("/polarion/testServletName/ui/regularUri"); + servlet = callServlet("/polarion/testServletName/ui/regularUri/someStyle.css"); verify(servlet, times(0)).serveGenericResource(any(), any()); verify(servlet, times(1)).serveResource(any(), any()); }