XSS Vulnerability fix in SharedContextRestController

Custom data validator used to secure this class

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

3 files changed, 200 insertions, 70 deletions

diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
index ba77c56f..9e3428e6 100644
--- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
+++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
@@ -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
index 00000000..2a26ab31
--- /dev/null
+++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java
@@ -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();
+       }
+}
diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
index 1607f423..49cccae5 100644
--- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
+++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
@@ -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());
+
+	}
+
 }