From b8a540c2aa08175b2d4c144c6a27d774c8e76acb Mon Sep 17 00:00:00 2001 From: Dominik Mizyn Date: Mon, 15 Jul 2019 12:32:27 +0200 Subject: [PATCH] XSS Vulnerability fix in AuxApiRequestMapperController Custom data validotor is used to valid incoming data. Issue-ID: OJSI-15 Signed-off-by: Dominik Mizyn Change-Id: I1b2a1fe0fcb6278a7b12017479243009142c2cbd --- .../model/ExternalSystemRoleApproval.java | 5 +- .../model/ExternalSystemUser.java | 10 +- .../controller/AuxApiRequestMapperController.java | 122 +++++++++-- .../onap/portalapp/portal/transport/Analytics.java | 7 +- .../AuxApiRequestMapperControllerTest.java | 230 ++++++++++++++++++++- 5 files changed, 352 insertions(+), 22 deletions(-) diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemRoleApproval.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemRoleApproval.java index 550d11df..49eb469c 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemRoleApproval.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemRoleApproval.java @@ -38,13 +38,14 @@ package org.onap.portalapp.externalsystemapproval.model; import java.io.Serializable; +import org.hibernate.validator.constraints.SafeHtml; public class ExternalSystemRoleApproval implements Serializable { private static final long serialVersionUID = 6048830318039958615L; - + @SafeHtml private String roleName; - + @SafeHtml public String getRoleName() { return roleName; } diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemUser.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemUser.java index cfe49267..fa6c04e1 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemUser.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/externalsystemapproval/model/ExternalSystemUser.java @@ -40,15 +40,17 @@ package org.onap.portalapp.externalsystemapproval.model; import java.util.ArrayList; import java.util.List; +import javax.validation.Valid; +import org.hibernate.validator.constraints.SafeHtml; public class ExternalSystemUser { - + @SafeHtml private String loginId; - + @SafeHtml private String applicationName; - + @SafeHtml private String myloginrequestId; - + @Valid private List roles; public ExternalSystemUser() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperController.java index fe2c349f..9ca88c0f 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperController.java @@ -36,6 +36,8 @@ */ package org.onap.portalapp.portal.controller; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.swagger.annotations.ApiOperation; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; @@ -47,10 +49,8 @@ import java.util.Map; import java.util.jar.Attributes; import java.util.regex.Matcher; import java.util.regex.Pattern; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import org.onap.aaf.cadi.aaf.AAFPermission; import org.onap.portalapp.annotation.ApiVersion; import org.onap.portalapp.externalsystemapproval.model.ExternalSystemUser; @@ -67,6 +67,8 @@ import org.onap.portalapp.portal.transport.EpNotificationItem; import org.onap.portalapp.portal.transport.FavoritesFunctionalMenuItemJson; import org.onap.portalapp.portal.transport.FunctionalMenuItem; import org.onap.portalapp.portal.transport.OnboardingApp; +import org.onap.portalapp.validation.DataValidator; +import org.onap.portalapp.validation.SecureString; import org.onap.portalsdk.core.domain.Role; import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate; import org.onap.portalsdk.core.onboarding.crossapi.PortalAPIResponse; @@ -76,6 +78,7 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.EnableAspectJAutoProxy; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; @@ -85,18 +88,15 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; -import com.fasterxml.jackson.databind.ObjectMapper; - -import io.swagger.annotations.ApiOperation; - @RestController @RequestMapping("/auxapi") -@org.springframework.context.annotation.Configuration +@Configuration @EnableAspectJAutoProxy @EPAuditLog public class AuxApiRequestMapperController implements ApplicationContextAware, BasicAuthenticationController { private static EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(AuxApiRequestMapperController.class); + private DataValidator dataValidator = new DataValidator(); ApplicationContext context = null; int minorVersion = 0; @@ -108,6 +108,13 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/user/{loginId}" }, method = RequestMethod.GET, produces = "application/json") public String getUser(HttpServletRequest request, HttpServletResponse response, @PathVariable("loginId") String loginId) throws Exception { + if (loginId!=null){ + SecureString secureLoginId = new SecureString(loginId); + if (!dataValidator.isValid(secureLoginId)) + return "Provided data is not valid"; + } + + Map res = getMethod(request, response); String answer = null; try { @@ -198,6 +205,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/function/{code}" }, method = RequestMethod.GET, produces = "application/json") public CentralV2RoleFunction getRoleFunction(HttpServletRequest request, HttpServletResponse response, @PathVariable("code") String code) throws Exception { + if (code!=null){ + SecureString secureCode = new SecureString(code); + if (!dataValidator.isValid(secureCode)) + return new CentralV2RoleFunction(); + } + Map res = getMethod(request, response); CentralV2RoleFunction roleFunction = null; try { @@ -214,13 +227,20 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B public PortalRestResponse saveRoleFunction(HttpServletRequest request, HttpServletResponse response, @RequestBody String roleFunc) throws Exception { PortalRestResponse result = null; + + if (roleFunc!=null){ + SecureString secureRoleFunc = new SecureString(roleFunc); + if(!dataValidator.isValid(secureRoleFunc)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Provided data is not valid", "Failed"); + } + Map res = getMethod(request, response); try { result = (PortalRestResponse) invokeMethod(res, request, response, roleFunc); return result; } catch (Exception e) { logger.error(EELFLoggerDelegate.errorLogger, "saveRoleFunction failed", e); - return new PortalRestResponse(PortalRestStatusEnum.ERROR, e.getMessage(), "Failed"); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, e.getMessage(), "Failed"); } } @@ -230,6 +250,13 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B public PortalRestResponse deleteRoleFunction(HttpServletRequest request, HttpServletResponse response, @PathVariable("code") String code) throws Exception { PortalRestResponse result = null; + + if (code!=null){ + SecureString secureCode = new SecureString(code); + if(!dataValidator.isValid(secureCode)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Provided data is not valid", "Failed"); + } + Map res = getMethod(request, response); try { result = (PortalRestResponse) invokeMethod(res, request, response, code); @@ -276,6 +303,14 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B public String getEcompUser(HttpServletRequest request, HttpServletResponse response, @PathVariable("loginId") String loginId) throws Exception { Map res = getMethod(request, response); + + if (loginId!=null){ + SecureString secureLoginId = new SecureString(loginId); + + if (!dataValidator.isValid(secureLoginId)) + return null; + } + String answer = null; try { answer = (String) invokeMethod(res, request, response, loginId); @@ -319,6 +354,14 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/extendSessionTimeOuts" }, method = RequestMethod.POST) public Boolean extendSessionTimeOuts(HttpServletRequest request, HttpServletResponse response, @RequestParam String sessionMap) throws Exception { + + if (sessionMap!=null){ + SecureString secureSessionMap = new SecureString(sessionMap); + if (!dataValidator.isValid(secureSessionMap)){ + return null; + } + } + Map res = getMethod(request, response); Boolean ans = null; try { @@ -347,6 +390,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @ApiOperation(value = "Accepts data from partner applications with web analytics data.", response = PortalAPIResponse.class) public PortalAPIResponse storeAnalyticsScript(HttpServletRequest request, HttpServletResponse response, @RequestBody Analytics analyticsMap) throws Exception { + + if (analyticsMap!=null){ + if (!dataValidator.isValid(analyticsMap)) + return new PortalAPIResponse(false, "analyticsScript is not valid"); + } + Map res = getMethod(request, response); PortalAPIResponse ans = new PortalAPIResponse(true, "error"); try { @@ -715,6 +764,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/userProfile" }, method = RequestMethod.POST, produces = "application/json") public PortalRestResponse postUserProfile(HttpServletRequest request, @RequestBody ExternalSystemUser extSysUser, HttpServletResponse response) { + + if (extSysUser!=null){ + if (!dataValidator.isValid(extSysUser)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ExternalSystemUser is not valid", "Failed"); + } + PortalRestResponse result = null; Map res = getMethod(request, response); try { @@ -731,6 +786,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/userProfile" }, method = RequestMethod.PUT, produces = "application/json") public PortalRestResponse putUserProfile(HttpServletRequest request, @RequestBody ExternalSystemUser extSysUser, HttpServletResponse response) { + + if (extSysUser!=null){ + if (!dataValidator.isValid(extSysUser)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ExternalSystemUser is not valid", "Failed"); + } + PortalRestResponse result = null; Map res = getMethod(request, response); try { @@ -747,6 +808,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/userProfile" }, method = RequestMethod.DELETE, produces = "application/json") public PortalRestResponse deleteUserProfile(HttpServletRequest request, @RequestBody ExternalSystemUser extSysUser, HttpServletResponse response) { + + if (extSysUser!=null){ + if (!dataValidator.isValid(extSysUser)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ExternalSystemUser is not valid", "Failed"); + } + PortalRestResponse result = null; Map res = getMethod(request, response); try { @@ -763,6 +830,13 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/ticketevent" }, method = RequestMethod.POST) public PortalRestResponse handleRequest(HttpServletRequest request, HttpServletResponse response, @RequestBody String ticketEventJson) throws Exception { + + if (ticketEventJson!=null){ + SecureString secureTicketEventJson = new SecureString(ticketEventJson); + if (!dataValidator.isValid(secureTicketEventJson)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ticketEventJson is not valid", "Failed"); + } + PortalRestResponse result = null; Map res = getMethod(request, response); try { @@ -780,6 +854,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @ResponseBody public PortalRestResponse postPortalAdmin(HttpServletRequest request, HttpServletResponse response, @RequestBody EPUser epUser) { + + if (epUser!=null){ + if (!dataValidator.isValid(epUser)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "EPUser is not valid", "Failed"); + } + PortalRestResponse result = null; Map res = getMethod(request, response); try { @@ -812,6 +892,12 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @ResponseBody public PortalRestResponse postOnboardAppExternal(HttpServletRequest request, HttpServletResponse response, @RequestBody OnboardingApp newOnboardApp) { + + if (newOnboardApp!=null){ + if (!dataValidator.isValid(newOnboardApp)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "OnboardingApp is not valid", "Failed"); + } + PortalRestResponse result = new PortalRestResponse<>(); Map res = getMethod(request, response); try { @@ -830,7 +916,13 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @ResponseBody public PortalRestResponse putOnboardAppExternal(HttpServletRequest request, HttpServletResponse response, @PathVariable("appId") Long appId, @RequestBody OnboardingApp oldOnboardApp) { - PortalRestResponse result = new PortalRestResponse<>(); + + if (oldOnboardApp!=null){ + if (!dataValidator.isValid(oldOnboardApp)) + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "OnboardingApp is not valid", "Failed"); + } + + PortalRestResponse result; Map res = getMethod(request, response); try { result = (PortalRestResponse) invokeMethod(res, request, response, appId, oldOnboardApp); @@ -845,12 +937,16 @@ public class AuxApiRequestMapperController implements ApplicationContextAware, B @RequestMapping(value = { "/v3/publishNotification" }, method = RequestMethod.POST, produces = "application/json") @ResponseBody public PortalAPIResponse publishNotification(HttpServletRequest request, - @RequestBody EpNotificationItem notificationItem, HttpServletResponse response) throws Exception { - PortalAPIResponse result = new PortalAPIResponse(true, "success"); + @RequestBody EpNotificationItem notificationItem, HttpServletResponse response) { + + if (notificationItem!=null){ + if (!dataValidator.isValid(notificationItem)) + return new PortalAPIResponse(false, "EpNotificationItem is not valid"); + } + Map res = getMethod(request, response); try { - result = (PortalAPIResponse) invokeMethod(res, request, response, notificationItem); - return result; + return (PortalAPIResponse) invokeMethod(res, request, response, notificationItem); } catch (Exception e) { logger.error(EELFLoggerDelegate.errorLogger, "publishNotification failed", e); return new PortalAPIResponse(false, e.getMessage()); diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/Analytics.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/Analytics.java index 2d85e8f2..f5ca1832 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/Analytics.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/Analytics.java @@ -38,14 +38,19 @@ package org.onap.portalapp.portal.transport; import com.fasterxml.jackson.annotation.JsonInclude; +import org.hibernate.validator.constraints.SafeHtml; @JsonInclude(JsonInclude.Include.NON_NULL) public class Analytics { - + @SafeHtml private String action; + @SafeHtml private String page; + @SafeHtml private String function; + @SafeHtml private String userid; + @SafeHtml private String type; public String getType() { diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperControllerTest.java index e7303313..5f49c744 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AuxApiRequestMapperControllerTest.java @@ -45,10 +45,8 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -68,6 +66,7 @@ import org.onap.portalapp.portal.transport.Analytics; import org.onap.portalapp.portal.transport.EpNotificationItem; import org.onap.portalapp.portal.transport.OnboardingApp; import org.onap.portalsdk.core.domain.Role; +import org.onap.portalsdk.core.onboarding.crossapi.PortalAPIResponse; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; @@ -114,6 +113,21 @@ public class AuxApiRequestMapperControllerTest { Mockito.when(mockedRequest.getMethod()).thenReturn("GET"); assertNull(auxApiRequestMapperController.getUser(mockedRequest, mockedResponse, "test12")); } + + @Test + public void getUserXSSTest() throws Exception { + Mockito.when(mockedRequest.getRequestURI()).thenReturn("/auxapi/v3/roles"); + Mockito.when(mockedRequest.getHeader("MinorVersion")).thenReturn("0"); + Map beans = new HashMap<>(); + beans.put("bean1", rolesController); + Mockito.when(context.getBeansWithAnnotation(ApiVersion.class)).thenReturn(beans); + PowerMockito.mockStatic(AopUtils.class); + Mockito.when(AopUtils.isAopProxy(Matchers.anyObject())).thenReturn(false); + Mockito.when(mockedRequest.getMethod()).thenReturn("GET"); + String expected = "Provided data is not valid"; + String actual = auxApiRequestMapperController.getUser(mockedRequest, mockedResponse, "“>"); + assertEquals(expected, actual); + } @Test public void getUserTestWithException() throws Exception { @@ -233,6 +247,7 @@ public class AuxApiRequestMapperControllerTest { assertNull(auxApiRequestMapperController.getRoleFunction(mockedRequest, mockedResponse, "test")); } + @Test public void saveRoleFunctionTest() throws Exception { Mockito.when(mockedRequest.getRequestURI()).thenReturn("/auxapi/v3/roleFunction"); @@ -247,6 +262,21 @@ public class AuxApiRequestMapperControllerTest { assertNotNull(response); } + @Test + public void saveRoleFunctionXSSTest() throws Exception { + Mockito.when(mockedRequest.getRequestURI()).thenReturn("/auxapi/v3/roleFunction"); + Mockito.when(mockedRequest.getHeader("MinorVersion")).thenReturn("0"); + Map beans = new HashMap<>(); + beans.put("bean1", rolesController); + Mockito.when(context.getBeansWithAnnotation(ApiVersion.class)).thenReturn(beans); + PowerMockito.mockStatic(AopUtils.class); + Mockito.when(AopUtils.isAopProxy(Matchers.anyObject())).thenReturn(false); + Mockito.when(mockedRequest.getMethod()).thenReturn("POST"); + PortalRestResponse actual = auxApiRequestMapperController.saveRoleFunction(mockedRequest, mockedResponse, ""); + PortalRestResponse expected = new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Provided data is not valid", "Failed"); + assertEquals(expected, actual); + } + @Test public void deleteRoleFunctionTest() throws Exception { Mockito.when(mockedRequest.getRequestURI()).thenReturn("/auxapi/v3/roleFunction/test"); @@ -260,6 +290,22 @@ public class AuxApiRequestMapperControllerTest { assertNull(auxApiRequestMapperController.deleteRoleFunction(mockedRequest, mockedResponse, "test")); } + @Test + public void deleteRoleFunctionXSSTest() throws Exception { + Mockito.when(mockedRequest.getRequestURI()).thenReturn("/auxapi/v3/roleFunction/test"); + Mockito.when(mockedRequest.getHeader("MinorVersion")).thenReturn("0"); + Map beans = new HashMap<>(); + beans.put("bean1", rolesController); + Mockito.when(context.getBeansWithAnnotation(ApiVersion.class)).thenReturn(beans); + PowerMockito.mockStatic(AopUtils.class); + Mockito.when(AopUtils.isAopProxy(Matchers.anyObject())).thenReturn(false); + Mockito.when(mockedRequest.getMethod()).thenReturn("DELETE"); + PortalRestResponse actual = auxApiRequestMapperController.deleteRoleFunction(mockedRequest, mockedResponse, + "''