From fc2201c27573aa872a1b0ade0078688bab5b9808 Mon Sep 17 00:00:00 2001 From: Sofien Haj Chedhli Date: Tue, 12 Dec 2023 18:30:55 +0100 Subject: [PATCH] fix : Addressing XSS vulnerability during application creation - EXO-67968 - Meeds-io/meeds#1441 (#307) Prior to this change, the XSS attacks were possible during the application creation process using an API tool, This change is going to prevent this type of attack by adding url validation with the same rules used on the application form drawer . --- .../appcenter/rest/ApplicationCenterREST.java | 4 ++ .../service/ApplicationCenterService.java | 13 ++++ .../service/ApplicationCenterServiceTest.java | 70 ++++++++++++------- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/app-center-services/src/main/java/org/exoplatform/appcenter/rest/ApplicationCenterREST.java b/app-center-services/src/main/java/org/exoplatform/appcenter/rest/ApplicationCenterREST.java index b51d659e2..1014c5d76 100644 --- a/app-center-services/src/main/java/org/exoplatform/appcenter/rest/ApplicationCenterREST.java +++ b/app-center-services/src/main/java/org/exoplatform/appcenter/rest/ApplicationCenterREST.java @@ -17,6 +17,7 @@ package org.exoplatform.appcenter.rest; import java.io.InputStream; +import java.net.MalformedURLException; import java.util.*; import javax.annotation.security.RolesAllowed; @@ -296,6 +297,9 @@ public Response getAppGeneralSettings() { public Response createApplication(@RequestBody(description = "Application to save", required = true) Application application) { try { appCenterService.createApplication(application); + } catch (MalformedURLException malformedURLException) { + LOG.error("The application URL or the application help page URL is malformed: {}", malformedURLException.getMessage()); + return Response.serverError().build(); } catch (ApplicationAlreadyExistsException e) { LOG.warn(e); return Response.serverError().build(); diff --git a/app-center-services/src/main/java/org/exoplatform/appcenter/service/ApplicationCenterService.java b/app-center-services/src/main/java/org/exoplatform/appcenter/service/ApplicationCenterService.java index 42660fc5f..76f2b35a8 100644 --- a/app-center-services/src/main/java/org/exoplatform/appcenter/service/ApplicationCenterService.java +++ b/app-center-services/src/main/java/org/exoplatform/appcenter/service/ApplicationCenterService.java @@ -18,10 +18,12 @@ import java.io.IOException; import java.io.InputStream; +import java.net.MalformedURLException; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.io.IOUtils; @@ -268,6 +270,10 @@ public Application createApplication(Application application) throws Exception { throw new ApplicationAlreadyExistsException("An application with same title already exists"); } + if (!isUrlValid(application.getUrl()) || (!application.getHelpPageURL().isBlank() && !isUrlValid(application.getHelpPageURL()))) { + throw new MalformedURLException(); + } + if (application.getPermissions() == null || application.getPermissions().isEmpty()) { application.setPermissions(DEFAULT_USERS_PERMISSION); } @@ -811,4 +817,11 @@ private List getApplications(String keyword, String username) throw return applications; } + private boolean isUrlValid(String url) { + //[-a-zA-Z0-9@:%._\\\\/+~#=] allowed characters + String regex = "(http(s)?:\\/\\/.)[-a-zA-Z0-9@:%._\\\\/+~#=]{2,256}"; + Pattern pattern = Pattern.compile(regex); + return url != null && !url.isBlank() && (url.startsWith("/portal/") || url.startsWith("./") || pattern.matcher(url).matches()); + } + } diff --git a/app-center-services/src/test/java/org/exoplatform/appcenter/service/ApplicationCenterServiceTest.java b/app-center-services/src/test/java/org/exoplatform/appcenter/service/ApplicationCenterServiceTest.java index 6125a8863..673e2ef38 100644 --- a/app-center-services/src/test/java/org/exoplatform/appcenter/service/ApplicationCenterServiceTest.java +++ b/app-center-services/src/test/java/org/exoplatform/appcenter/service/ApplicationCenterServiceTest.java @@ -20,6 +20,7 @@ import java.io.FileNotFoundException; import java.io.InputStream; +import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Collection; @@ -145,7 +146,7 @@ public void testCreateApplication() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -177,6 +178,27 @@ public void testCreateApplication() throws Exception { } catch (ApplicationAlreadyExistsException e) { // Expected } + // + application.setUrl(""); + application.setTitle("applicationWithEmptyUrl"); + assertThrows(MalformedURLException.class, () -> applicationCenterService.createApplication(application)); + // + application.setUrl("./url"); + application.setHelpPageURL("javascript:alert('XSS')"); + application.setTitle("applicationWithMalFormedHelpPageUrl"); + assertThrows(MalformedURLException.class, () -> applicationCenterService.createApplication(application)); + // + application.setUrl("/portal/url"); + application.setHelpPageURL(""); + application.setTitle("applicationWithValidUrlAndEmptyHelpPageUrl"); + storedApplication = applicationCenterService.createApplication(application); + assertNotNull(storedApplication); + // + application.setUrl("https://applictaion/url"); + application.setHelpPageURL("https://help/page/url"); + application.setTitle("applicationWithValidUrlAndHelpPageUrl"); + storedApplication = applicationCenterService.createApplication(application); + assertNotNull(storedApplication); } @Test @@ -197,7 +219,7 @@ public void testUpdateApplication() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -265,9 +287,9 @@ public void testUpdateApplication() throws Exception { storedApplication = applicationCenterService.updateApplication(application, ADMIN_USERNAME); assertEquals(ApplicationCenterService.DEFAULT_USERS_PERMISSION, storedApplication.getPermissions().get(0)); - application.setUrl("url2"); + application.setUrl("./url2"); storedApplication = applicationCenterService.updateApplication(application, ADMIN_USERNAME); - assertEquals("url2", storedApplication.getUrl()); + assertEquals("./url2", storedApplication.getUrl()); } @Test @@ -303,7 +325,7 @@ public void testDeleteApplication() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -321,7 +343,7 @@ public void testDeleteApplication() throws Exception { application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -434,7 +456,7 @@ public void testAddFavoriteApplication() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -485,7 +507,7 @@ public void testDeleteFavoriteApplication() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -526,7 +548,7 @@ public void testGetApplicationsList() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -543,7 +565,7 @@ public void testGetApplicationsList() throws Exception { Application application2 = new Application(null, "title2", - "url2", + "./url2", "", 0L, 0L, @@ -593,7 +615,7 @@ public void testGetApplicationsList() throws Exception { public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { Application application1 = new Application(null, "title1", - "url1", + "./url1", "", 0L, 0L, @@ -609,7 +631,7 @@ public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { Application application2 = new Application(null, "title2", - "url2", + "./url2", "", 0L, 0L, @@ -625,7 +647,7 @@ public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { Application application3 = new Application(null, "title3", - "url3", + "./url3", "", 0L, 0L, @@ -641,7 +663,7 @@ public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { Application application4 = new Application(null, "title4", - "url4", + "./url4", "", 0L, 0L, @@ -657,7 +679,7 @@ public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { Application application5 = new Application(null, "title5", - "url5", + "./url5", "", 0L, 0L, @@ -690,7 +712,7 @@ public void testGetMandatoryAndFavoriteApplicationsList() throws Exception { public void testUpdateFavoriteApplicationOrder() throws Exception { Application application1 = new Application(null, "title3", - "url3", + "./url3", "", 0L, 0L, @@ -706,7 +728,7 @@ public void testUpdateFavoriteApplicationOrder() throws Exception { Application application2 = new Application(null, "title5", - "url5", + "./url5", "", 0L, 0L, @@ -775,7 +797,7 @@ public void testGetAuthorizedApplicationsList() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 0L, 0L, @@ -793,7 +815,7 @@ public void testGetAuthorizedApplicationsList() throws Exception { Application application2 = new Application(null, "title2", - "url2", + "./url2", "", 0L, 0L, @@ -898,7 +920,7 @@ public void testGetLastUpdated() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 5L, 0L, @@ -948,7 +970,7 @@ public void testGetImageStream() throws Exception { Application application = new Application(null, "title", - "url", + "./url", "", 5L, 0L, @@ -1005,7 +1027,7 @@ public void testAddApplicationPlugin() throws FileStorageException { Application application = new Application(null, "title", - "url", + "./url", "", 5L, 0L, @@ -1159,7 +1181,7 @@ public void testEnableDisableApplication() { applicationParam.setName("application"); applicationParam.setObject(new Application(null, "title", - "url", + "./url", "", 5L, 0L, @@ -1241,7 +1263,7 @@ public void testUpdateApplicationBySystem() { } Application application = new Application(null, "title", - "url", + "./url", "", 5L, 0L,