From: Sunder Tattavarada Date: Fri, 14 Jun 2019 16:18:10 +0000 (+0000) Subject: Merge "XSS Vulnerability fix in DashboardSearchResultController" X-Git-Tag: 3.2.0~294 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=portal.git;a=commitdiff_plain;h=9d730f0c98561e566be9adba12c0b044f5924a9f;hp=3b4d9e772bc96effe948abf4f8e34737a1030148 Merge "XSS Vulnerability fix in DashboardSearchResultController" --- diff --git a/INFO.yaml b/INFO.yaml index 5ce7de9c..7f6ab339 100644 --- a/INFO.yaml +++ b/INFO.yaml @@ -41,5 +41,10 @@ committers: company: 'ATT' id: 'st782s' timezone: 'America/New_York' + - name: 'Lorraine A Welch' + email: 'lb2391@att.com' + company: 'ATT' + id: 'lorraineawelch' + timezone: 'America/New_York' tsc: approval: 'https://lists.onap.org/pipermail/onap-tsc' diff --git a/docs/release-notes.rst b/docs/release-notes.rst index 457819bc..a1b6e09c 100644 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -35,9 +35,15 @@ We worked on SDK upgrade to integrate with AAF. We partially implemented multi-l *Fixed Security Issues* *Known Security Issues* - * In defult deployment PORTAL (portal-app) exposes HTTP port 8989 outside of cluster. [`OJSI-97 `_] - * In defult deployment PORTAL (portal-app) exposes HTTP port 30215 outside of cluster. [`OJSI-105 `_] - * In defult deployment PORTAL (portal-sdk) exposes HTTP port 30212 outside of cluster. [`OJSI-106 `_] + + * CVE-2019-12317 - Number of XSS vulnerabilities in Portal [`OJSI-15 `_] + * CVE-2019-12122 - ONAP Portal allows to retrieve password of currently active user [`OJSI-65 `_] + * CVE-2019-12121 - ONAP Portal is vulnerable for Padding Oracle attack [`OJSI-92 `_] + * In defult deployment PORTAL (portal-app) exposes HTTP port 8989 outside of cluster. [`OJSI-97 `_] + * In defult deployment PORTAL (portal-app) exposes HTTP port 30215 outside of cluster. [`OJSI-105 `_] + * In defult deployment PORTAL (portal-sdk) exposes HTTP port 30212 outside of cluster. [`OJSI-106 `_] + * CVE-2019-12318 - Number of SQL Injections in Portal [`OJSI-174 `_] + * Portal stores users passwords encrypted instead of hashed [`OJSI-190 `_] *Known Vulnerabilities in Used Modules* @@ -53,7 +59,8 @@ Quick Links: **Upgrade Notes** * For https Apps onboarded to portal, a certificate has to be downloaded in the browser when first trying to access the landing page of the App. * For onboarded Apps using http (since Portal is using https) the browser asks the user to click to Proceed to the unsafe URL. - * For onboarded Apps using http the icon in the URL bar will appear red, click on it and allow unsafe scripts. + * For onboarded Apps using http the icon in the URL bar will appear red, click on it and allow unsafe scripts. + * The first time some apps are selected in the Applications panel, an error stating the webpage might be temporarily down, copy the presented URL to a new browser; once that is done, the application will open in the Portal. **Deprecation Notes** diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImpl.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImpl.java index 5d9761ce..aaaf91bd 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImpl.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImpl.java @@ -176,10 +176,10 @@ public class UserRolesCommonServiceImpl { * * @param userId */ - protected void createLocalUserIfNecessary(String userId) { + protected boolean createLocalUserIfNecessary(String userId) { if (StringUtils.isEmpty(userId)) { logger.error(EELFLoggerDelegate.errorLogger, "createLocalUserIfNecessary : empty userId!"); - return; + return false; } Session localSession = null; Transaction transaction = null; @@ -188,7 +188,10 @@ public class UserRolesCommonServiceImpl { transaction = localSession.beginTransaction(); @SuppressWarnings("unchecked") List userList = localSession - .createQuery("from " + EPUser.class.getName() + " where orgUserId='" + userId + "'").list(); + .createQuery("from :name where orgUserId=:userId") + .setParameter("name",EPUser.class.getName()) + .setParameter("userId",userId) + .list(); if (userList.size() == 0) { EPUser client = searchService.searchUserByUserId(userId); if (client == null) { @@ -202,9 +205,11 @@ public class UserRolesCommonServiceImpl { } } transaction.commit(); + return true; } catch (Exception e) { EPLogUtil.logEcompError(logger, EPAppMessagesEnum.BeDaoSystemError, e); EcompPortalUtils.rollbackTransaction(transaction, "searchOrCreateUser rollback, exception = " + e); + return false; } finally { EcompPortalUtils.closeLocalSession(localSession, "searchOrCreateUser"); } diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImplTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImplTest.java index c907a6e5..82b902a1 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImplTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/UserRolesCommonServiceImplTest.java @@ -55,6 +55,7 @@ import java.util.TreeSet; import javax.servlet.http.HttpServletResponse; import org.apache.cxf.transport.http.HTTPException; +import org.drools.core.command.assertion.AssertEquals; import org.hibernate.Query; import org.hibernate.SQLQuery; import org.hibernate.Session; @@ -237,6 +238,31 @@ public class UserRolesCommonServiceImplTest { return mockRoleInAppForUserList; } + @SuppressWarnings("unchecked") + @Test + public void checkTheProtectionAgainstSQLInjection() throws Exception { + EPUser user = mockUser.mockEPUser(); + user.setId(1l); + user.setOrgId(2l); + Query epUserQuery = Mockito.mock(Query.class); + List mockEPUserList = new ArrayList<>(); + mockEPUserList.add(user); + + // test with SQL injection, should return false + Mockito.when(session.createQuery("from :name where orgUserId=:userId")).thenReturn(epUserQuery); + Mockito.when(epUserQuery.setParameter("name",EPUser.class.getName())).thenReturn(epUserQuery); + Mockito.when(epUserQuery.setParameter("userId",user.getOrgUserId() + "; select * from " + EPUser.class.getName() +";")).thenReturn(epUserQuery); + boolean ret = userRolesCommonServiceImpl.createLocalUserIfNecessary(user.getOrgUserId()); + assertFalse(ret); + + // test without SQL injection, should return true + Mockito.when(session.createQuery("from :name where orgUserId=:userId")).thenReturn(epUserQuery); + Mockito.when(epUserQuery.setParameter("name",EPUser.class.getName())).thenReturn(epUserQuery); + Mockito.when(epUserQuery.setParameter("userId",user.getOrgUserId())).thenReturn(epUserQuery); + ret = userRolesCommonServiceImpl.createLocalUserIfNecessary(user.getOrgUserId()); + assertTrue(ret); + } + @SuppressWarnings("unchecked") @Test public void getAppRolesForUserNonCentralizedForPortal() throws Exception { diff --git a/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/controller/LoginController.java b/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/controller/LoginController.java index 0ba7bdc6..56064b99 100644 --- a/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/controller/LoginController.java +++ b/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/controller/LoginController.java @@ -39,6 +39,7 @@ package org.onap.portalapp.controller; import static com.att.eelf.configuration.Configuration.MDC_KEY_REQUEST_ID; +import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.net.URLDecoder; @@ -68,8 +69,10 @@ import org.onap.portalsdk.core.menu.MenuProperties; import org.onap.portalsdk.core.util.SystemProperties; import org.slf4j.MDC; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Controller; import org.springframework.util.StopWatch; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.ResponseBody; @@ -409,4 +412,9 @@ public class LoginController extends EPUnRestrictedBaseController implements Log this.sharedContextService = sharedContextService; } + @ExceptionHandler(Exception.class) + protected void handleBadRequests(Exception e, HttpServletResponse response) throws IOException { + logger.warn(EELFLoggerDelegate.errorLogger, "Handling bad request", e); + response.sendError(HttpStatus.BAD_REQUEST.value()); + } } diff --git a/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/filter/SecurityXssFilter.java b/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/filter/SecurityXssFilter.java index 25eee828..703019f9 100644 --- a/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/filter/SecurityXssFilter.java +++ b/ecomp-portal-BE-os/src/main/java/org/onap/portalapp/filter/SecurityXssFilter.java @@ -1,9 +1,9 @@ - /*- * ============LICENSE_START========================================== * ONAP Portal * =================================================================== * Copyright © 2017 AT&T Intellectual Property. All rights reserved. + * Modifications Copyright (c) 2019 Samsung * =================================================================== * * Unless otherwise specified, all software contained herein is licensed @@ -36,6 +36,7 @@ * * */ + package org.onap.portalapp.filter; import java.io.BufferedReader; @@ -48,7 +49,6 @@ import java.util.Enumeration; import javax.servlet.FilterChain; import javax.servlet.ReadListener; -import javax.servlet.ServletException; import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; @@ -62,7 +62,7 @@ import org.springframework.web.filter.OncePerRequestFilter; public class SecurityXssFilter extends OncePerRequestFilter { - private EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(SecurityXssFilter.class); + private EELFLoggerDelegate sxLogger = EELFLoggerDelegate.getLogger(SecurityXssFilter.class); private static final String APPLICATION_JSON = "application/json"; @@ -120,40 +120,47 @@ public class SecurityXssFilter extends OncePerRequestFilter { @Override public void setReadListener(ReadListener readListener) { - + // do nothing } - } } @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) - throws ServletException, IOException { + throws IOException { StringBuilder requestURL = new StringBuilder(request.getRequestURL().toString()); - String queryString = request.getQueryString(); - String requestUrl = ""; - if (queryString == null) { - requestUrl = requestURL.toString(); - } else { - requestUrl = requestURL.append('?').append(queryString).toString(); - } - validateRequest(requestUrl, response); + String queryString = request.getQueryString(); + String requestUrl; + + if (queryString == null) { + requestUrl = requestURL.toString(); + } else { + requestUrl = requestURL.append('?').append(queryString).toString(); + } + + validateRequest(requestUrl, response); StringBuilder headerValues = new StringBuilder(); Enumeration headerNames = request.getHeaderNames(); + while (headerNames.hasMoreElements()) { - String key = (String) headerNames.nextElement(); + String key = headerNames.nextElement(); String value = request.getHeader(key); headerValues.append(value); } + validateRequest(headerValues.toString(), response); + if (validateRequestType(request)) { request = new RequestWrapper(request); String requestData = IOUtils.toString(request.getInputStream(), StandardCharsets.UTF_8.toString()); validateRequest(requestData, response); - filterChain.doFilter(request, response); + } - } else { + try { filterChain.doFilter(request, response); + } catch (Exception e) { + sxLogger.warn(EELFLoggerDelegate.errorLogger, "Handling bad request", e); + response.sendError(org.springframework.http.HttpStatus.BAD_REQUEST.value(), "Handling bad request"); } } @@ -171,9 +178,8 @@ public class SecurityXssFilter extends OncePerRequestFilter { throw new SecurityException(ERROR_BAD_REQUEST); } } catch (Exception e) { - logger.error(EELFLoggerDelegate.errorLogger, "doFilterInternal() failed due to BAD_REQUEST", e); + sxLogger.error(EELFLoggerDelegate.errorLogger, "doFilterInternal() failed due to BAD_REQUEST", e); response.getWriter().close(); - return; } } -} \ No newline at end of file +}