XSS Vulnerability fix in SharedContextRestController 29/91329/1
authorDominik Mizyn <d.mizyn@samsung.com>
Thu, 6 Jun 2019 10:28:51 +0000 (12:28 +0200)
committerDominik Mizyn <d.mizyn@samsung.com>
Fri, 12 Jul 2019 10:01:35 +0000 (12:01 +0200)
Custom data validator used to secure this class

Issue-ID: OJSI-15
Signed-off-by: Dominik Mizyn <d.mizyn@samsung.com>
Change-Id: I231731b9deb60310b698d70179cddd471cffd7fb

ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java [new file with mode: 0644]
ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java

index ba77c56..9e3428e 100644 (file)
@@ -48,10 +48,13 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.onap.portalapp.controller.EPRestrictedRESTfulBaseController;
 import org.onap.portalapp.portal.domain.SharedContext;
+import org.onap.portalapp.portal.exceptions.NotValidDataException;
 import org.onap.portalapp.portal.logging.aop.EPAuditLog;
 import org.onap.portalapp.portal.service.SharedContextService;
 import org.onap.portalapp.portal.utils.EPCommonSystemProperties;
 import org.onap.portalapp.portal.utils.PortalConstants;
+import org.onap.portalapp.validation.DataValidator;
+import org.onap.portalapp.validation.SecureString;
 import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Configuration;
@@ -85,33 +88,20 @@ import io.swagger.annotations.ApiOperation;
 @EnableAspectJAutoProxy
 @EPAuditLog
 public class SharedContextRestController extends EPRestrictedRESTfulBaseController {
+       private static final DataValidator dataValidator = new DataValidator();
+       private static final EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(SharedContextRestController.class);
+       private static final ObjectMapper mapper = new ObjectMapper();
 
-       /**
-        * Model for a one-element JSON object returned by many methods.
-        */
-       class SharedContextJsonResponse {
-               String response;
-       }
-
-       /**
-        * Access to the database
-        */
-       @Autowired
        private SharedContextService contextService;
 
-       /**
-        * Logger for debug etc.
-        */
-       private EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(SharedContextRestController.class);
-
-       /**
-        * Reusable JSON (de)serializer
-        */
-       private final ObjectMapper mapper = new ObjectMapper();
+       @Autowired
+       public SharedContextRestController(SharedContextService contextService) {
+               this.contextService = contextService;
+       }
 
        /**
         * Gets a value for the specified context and key (RESTful service method).
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param context_id
@@ -127,13 +117,18 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
        @RequestMapping(value = { "/get" }, method = RequestMethod.GET, produces = "application/json")
        public String getContext(HttpServletRequest request, @RequestParam String context_id, @RequestParam String ckey)
                        throws Exception {
-
                logger.debug(EELFLoggerDelegate.debugLogger, "getContext for ID " + context_id + ", key " + ckey);
                if (context_id == null || ckey == null)
                        throw new Exception("Received null for context_id and/or ckey");
+               SecureString secureContextId = new SecureString(context_id);
+               SecureString secureCKey = new SecureString(ckey);
+
+               if(!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey)){
+                       throw new NotValidDataException("Received not valid for context_id and/or ckey");
+               }
 
                SharedContext context = contextService.getSharedContext(context_id, ckey);
-               String jsonResponse = "";
+               String jsonResponse;
                if (context == null)
                        jsonResponse = convertResponseToJSON(context);
                else
@@ -144,7 +139,7 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
 
        /**
         * Gets user information for the specified context (RESTful service method).
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param context_id
@@ -162,8 +157,11 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                logger.debug(EELFLoggerDelegate.debugLogger, "getUserContext for ID " + context_id);
                if (context_id == null)
                        throw new Exception("Received null for context_id");
+               SecureString secureContextId = new SecureString(context_id);
+               if (!dataValidator.isValid(secureContextId))
+                       throw new NotValidDataException("context_id is not valid");
 
-               List<SharedContext> listSharedContext = new ArrayList<SharedContext>();
+               List<SharedContext> listSharedContext = new ArrayList<>();
                SharedContext firstNameContext = contextService.getSharedContext(context_id,
                                EPCommonSystemProperties.USER_FIRST_NAME);
                SharedContext lastNameContext = contextService.getSharedContext(context_id,
@@ -179,14 +177,13 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                        listSharedContext.add(emailContext);
                if (orgUserIdContext != null)
                        listSharedContext.add(orgUserIdContext);
-               String jsonResponse = convertResponseToJSON(listSharedContext);
-               return jsonResponse;
+               return convertResponseToJSON(listSharedContext);
        }
 
        /**
         * Tests for presence of the specified key in the specified context (RESTful
         * service method).
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param context_id
@@ -208,19 +205,24 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                if (context_id == null || ckey == null)
                        throw new Exception("Received null for contextId and/or key");
 
+               SecureString secureContextId = new SecureString(context_id);
+               SecureString secureCKey = new SecureString(ckey);
+
+               if (!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey))
+                       throw new NotValidDataException("Not valid data for contextId and/or key");
+
                String response = null;
                SharedContext context = contextService.getSharedContext(context_id, ckey);
                if (context != null)
                        response = "exists";
 
-               String jsonResponse = convertResponseToJSON(response);
-               return jsonResponse;
+               return convertResponseToJSON(response);
        }
 
        /**
         * Removes the specified key in the specified context (RESTful service
         * method).
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param context_id
@@ -242,6 +244,12 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                if (context_id == null || ckey == null)
                        throw new Exception("Received null for contextId and/or key");
 
+               SecureString secureContextId = new SecureString(context_id);
+               SecureString secureCKey = new SecureString(ckey);
+
+               if (!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey))
+                       throw new NotValidDataException("Not valid data for contextId and/or key");
+
                SharedContext context = contextService.getSharedContext(context_id, ckey);
                String response = null;
                if (context != null) {
@@ -249,14 +257,13 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                        response = "removed";
                }
 
-               String jsonResponse = convertResponseToJSON(response);
-               return jsonResponse;
+               return convertResponseToJSON(response);
        }
 
        /**
         * Clears all key-value pairs in the specified context (RESTful service
         * method).
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param context_id
@@ -275,16 +282,20 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                if (context_id == null)
                        throw new Exception("clearContext: Received null for contextId");
 
+               SecureString secureContextId = new SecureString(context_id);
+
+               if (!dataValidator.isValid(secureContextId))
+                       throw new NotValidDataException("Not valid data for contextId");
+
                int count = contextService.deleteSharedContexts(context_id);
-               String jsonResponse = convertResponseToJSON(Integer.toString(count));
-               return jsonResponse;
+               return convertResponseToJSON(Integer.toString(count));
        }
 
        /**
         * Sets a context value for the specified context and key (RESTful service
         * method). Creates the context if no context with the specified ID-key pair
         * exists, overwrites the value if it exists already.
-        * 
+        *
         * @param request
         *            HTTP servlet request
         * @param userJson
@@ -302,6 +313,11 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
        @ApiOperation(value = "Sets a context value for the specified context and key. Creates the context if no context with the specified ID-key pair exists, overwrites the value if it exists already.", response = SharedContextJsonResponse.class)
        @RequestMapping(value = { "/set" }, method = RequestMethod.POST, produces = "application/json")
        public String setContext(HttpServletRequest request, @RequestBody String userJson) throws Exception {
+               if (userJson !=null){
+               SecureString secureUserJson = new SecureString(userJson);
+               if (!dataValidator.isValid(secureUserJson))
+                       throw new NotValidDataException("Not valid data for userJson");
+               }
 
                @SuppressWarnings("unchecked")
                Map<String, Object> userData = mapper.readValue(userJson, Map.class);
@@ -313,7 +329,7 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                        throw new Exception("setContext: received null for contextId and/or key");
 
                logger.debug(EELFLoggerDelegate.debugLogger, "setContext: ID " + contextId + ", key " + key + "->" + value);
-               String response = null;
+               String response;
                SharedContext existing = contextService.getSharedContext(contextId, key);
                if (existing == null) {
                        contextService.addSharedContext(contextId, key, value);
@@ -322,53 +338,49 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
                        contextService.saveSharedContext(existing);
                }
                response = existing == null ? "added" : "replaced";
-               String jsonResponse = convertResponseToJSON(response);
-               return jsonResponse;
+               return convertResponseToJSON(response);
        }
 
        /**
         * Creates a two-element JSON object tagged "response".
-        * 
+        *
         * @param responseBody
         * @return JSON object as String
         * @throws JsonProcessingException
         */
        private String convertResponseToJSON(String responseBody) throws JsonProcessingException {
-               Map<String, String> responseMap = new HashMap<String, String>();
+               Map<String, String> responseMap = new HashMap<>();
                responseMap.put("response", responseBody);
-               String response = mapper.writeValueAsString(responseMap);
-               return response;
+               return mapper.writeValueAsString(responseMap);
        }
 
        /**
         * Converts a list of SharedContext objects to a JSON array.
-        * 
+        *
         * @param contextList
         * @return JSON array as String
         * @throws JsonProcessingException
         */
        private String convertResponseToJSON(List<SharedContext> contextList) throws JsonProcessingException {
-               String jsonArray = mapper.writeValueAsString(contextList);
-               return jsonArray;
+               return mapper.writeValueAsString(contextList);
        }
 
        /**
         * Creates a JSON object with the content of the shared context; null is ok.
-        * 
+        *
         * @param context
         * @return tag "response" with collection of context object's fields
         * @throws JsonProcessingException
         */
        private String convertResponseToJSON(SharedContext context) throws JsonProcessingException {
-               Map<String, Object> responseMap = new HashMap<String, Object>();
+               Map<String, Object> responseMap = new HashMap<>();
                responseMap.put("response", context);
-               String responseBody = mapper.writeValueAsString(responseMap);
-               return responseBody;
+               return mapper.writeValueAsString(responseMap);
        }
 
        /**
         * Handles any exception thrown by a method in this controller.
-        * 
+        *
         * @param e
         *            Exception
         * @param response
@@ -382,3 +394,7 @@ public class SharedContextRestController extends EPRestrictedRESTfulBaseControll
        }
 
 }
+class SharedContextJsonResponse {
+       String response;
+}
+
diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java
new file mode 100644 (file)
index 0000000..2a26ab3
--- /dev/null
@@ -0,0 +1,51 @@
+/*-
+ * ============LICENSE_START==========================================
+ * ONAP Portal
+ * ===================================================================
+ * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved.
+ * ===================================================================
+ *
+ * Unless otherwise specified, all software contained herein is licensed
+ * under the Apache License, Version 2.0 (the "License");
+ * you may not use this software except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *             http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * Unless otherwise specified, all documentation contained herein is licensed
+ * under the Creative Commons License, Attribution 4.0 Intl. (the "License");
+ * you may not use this documentation except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *             https://creativecommons.org/licenses/by/4.0/
+ *
+ * Unless required by applicable law or agreed to in writing, documentation
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * ============LICENSE_END============================================
+ *
+ *
+ */
+
+package org.onap.portalapp.portal.exceptions;
+
+public class NotValidDataException extends Exception {
+
+       public NotValidDataException(String msg) {
+              super(msg);
+       }
+
+       @Override
+       public String toString() {
+              return "NotValidDataException{}: " + this.getMessage();
+       }
+}
index 1607f42..49cccae 100644 (file)
@@ -38,24 +38,19 @@ package org.onap.portalapp.portal.controller;
  */
 
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
-import java.io.IOException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.drools.core.command.assertion.AssertEquals;
 import org.json.JSONObject;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -64,24 +59,15 @@ import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
-import org.onap.portalapp.portal.controller.SharedContextRestClient;
-import org.onap.portalapp.portal.controller.SharedContextTestProperties;
 import org.onap.portalapp.portal.core.MockEPUser;
-import org.onap.portalapp.portal.domain.CentralV2RoleFunction;
 import org.onap.portalapp.portal.domain.SharedContext;
+import org.onap.portalapp.portal.exceptions.NotValidDataException;
 import org.onap.portalapp.portal.framework.MockitoTestSuite;
-import org.onap.portalapp.portal.scheduler.SchedulerProperties;
 import org.onap.portalapp.portal.service.SharedContextService;
 import org.onap.portalapp.portal.utils.EPCommonSystemProperties;
-import org.onap.portalsdk.core.util.SystemProperties;
-import org.onap.portalsdk.core.web.support.UserUtils;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
-import org.springframework.beans.factory.annotation.Autowired;
-
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
 
 /**
  * Tests the endpoints exposed by the Shared Context controller in Portal.
@@ -95,7 +81,7 @@ public class SharedContextRestControllerTest {
        SharedContextService contextService;
 
        @InjectMocks
-       SharedContextRestController sharedContextRestController=new SharedContextRestController();
+       SharedContextRestController sharedContextRestController=new SharedContextRestController(contextService);
        
        @Before
        public void setup() {
@@ -220,11 +206,31 @@ public class SharedContextRestControllerTest {
        public void getContextTestWithException() throws Exception{
                sharedContextRestController.getContext(mockedRequest, null,null);
        }
+
+       @Test(expected=NotValidDataException.class)
+       public void getContextTestNotValidDataException() throws Exception{
+               sharedContextRestController.getContext(mockedRequest, "<script>alert(\"hellox worldss\");</script>","test");
+       }
+
+       @Test(expected=NotValidDataException.class)
+       public void getContextTest2NotValidDataException() throws Exception{
+               sharedContextRestController.getContext(mockedRequest, "test","“><script>alert(“XSS”)</script>");
+       }
+
+       @Test(expected=NotValidDataException.class)
+       public void getContextTest3NotValidDataException() throws Exception{
+               sharedContextRestController.getContext(mockedRequest, "<ScRipT>alert(\"XSS\");</ScRipT>","“><script>alert(“XSS”)</script>");
+       }
        
-       @Test(expected=Exception.class)
+       @Test(expected= Exception.class)
        public void getUserContextTest() throws Exception{
                sharedContextRestController.getUserContext(mockedRequest, null);
        }
+
+       @Test(expected= NotValidDataException.class)
+       public void getUserContextXSSTest() throws Exception{
+               sharedContextRestController.getUserContext(mockedRequest, "<svg><script x:href='https://dl.dropbox.com/u/13018058/js.js' {Opera}");
+       }
        
        @Test
        public void getUserContextTestWithContext() throws Exception{
@@ -257,6 +263,16 @@ public class SharedContextRestControllerTest {
                Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
                sharedContextRestController.checkContext(mockedRequest, null,null);
        }
+
+       @Test(expected=NotValidDataException.class)
+       public void checkContextTestWithContextXSSl() throws Exception{
+               SharedContext sharedContext=new SharedContext();
+               sharedContext.setContext_id("test_contextid");
+               sharedContext.setCkey("test_ckey");
+               Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+               sharedContextRestController.checkContext(mockedRequest,
+                       "<ScRipT 5-0*3+9/3=>prompt(1)</ScRipT giveanswerhere=?","<script>alert(123);</script>");
+       }
        
        @Test
        public void removeContextTest() throws Exception{
@@ -283,6 +299,20 @@ public class SharedContextRestControllerTest {
                assertNotNull(actual);
 
        }
+
+       @Test(expected=NotValidDataException.class)
+       public void removeContextTestWithContextXSS() throws Exception{
+               SharedContext sharedContext=new SharedContext();
+               sharedContext.setContext_id("test_contextid");
+               sharedContext.setCkey("test_ckey");
+               Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+
+               //Mockito.when(contextService.deleteSharedContext(sharedContext));
+               String actual=sharedContextRestController.removeContext(mockedRequest,
+                       "<script>alert(“XSS”)</script> ","<script>alert(/XSS/)</script>");
+               assertNotNull(actual);
+
+       }
        
        @Test(expected=Exception.class)
        public void clearContextTestwithContextIdNull() throws Exception{
@@ -293,6 +323,16 @@ public class SharedContextRestControllerTest {
                assertNotNull(actual);
 
        }
+
+       @Test(expected=NotValidDataException.class)
+       public void clearContextTestwithContextXSS() throws Exception{
+
+               Mockito.when(contextService.deleteSharedContexts(Matchers.any())).thenReturn(12);
+
+               String actual=sharedContextRestController.clearContext(mockedRequest,"<script>alert(123)</script>");
+               assertNotNull(actual);
+
+       }
        
        @Test
        public void clearContextTest() throws Exception{
@@ -350,4 +390,27 @@ public class SharedContextRestControllerTest {
                String actual=sharedContextRestController.setContext(mockedRequest,testUserJson.toString());
 
        }
+
+       @Test(expected=NotValidDataException.class)
+       public void setContextTestWithContextXSS() throws Exception{
+               ObjectMapper mapper = new ObjectMapper();
+               Map<String, Object> userData = new HashMap<String, Object>();
+               userData.put("context_id", "test_contextId");
+               userData.put("ckey", "<script>alert(‘XSS’)</script>");
+               userData.put("cvalue", "test_cvalue");
+               //String testUserJson=Matchers.anyString();
+               JSONObject testUserJson = new JSONObject();
+               testUserJson.put("context_id", "test1ContextId");
+               testUserJson.put("ckey", "testCkey");
+               testUserJson.put("cvalue", "<script>alert(‘XSS’)</script>");
+               Map<String, Object> userData1 = mapper.readValue(testUserJson.toString(), Map.class);
+               SharedContext sharedContext=new SharedContext();
+               sharedContext.setContext_id("test_contextid");
+               sharedContext.setCkey("test_ckey");
+               Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+               // Mockito.when(mapper.readValue("true", Map.class)).thenReturn(userData);
+               String actual=sharedContextRestController.setContext(mockedRequest,testUserJson.toString());
+
+       }
+
 }