diff --git a/src/java/fr/paris/lutece/portal/web/style/ThemesJspBean.java b/src/java/fr/paris/lutece/portal/web/style/ThemesJspBean.java index 19222f0b89..34e69183e7 100644 --- a/src/java/fr/paris/lutece/portal/web/style/ThemesJspBean.java +++ b/src/java/fr/paris/lutece/portal/web/style/ThemesJspBean.java @@ -40,6 +40,7 @@ import fr.paris.lutece.portal.service.portal.ThemesService; import fr.paris.lutece.portal.service.template.AppTemplateService; import fr.paris.lutece.portal.service.util.AppPathService; +import fr.paris.lutece.portal.service.util.AppPropertiesService; import fr.paris.lutece.portal.web.admin.AdminFeaturesPageJspBean; import fr.paris.lutece.portal.web.constants.Messages; import fr.paris.lutece.util.html.HtmlTemplate; @@ -138,7 +139,8 @@ public String doModifyUserTheme( HttpServletRequest request, HttpServletResponse String strTheme = request.getParameter( PARAMETER_THEME ); String strForwardUrl = request.getParameter( PARAMETER_URL ); - if ( !SecurityUtil.containsCleanParameters( request ) ) + if ( !SecurityUtil.containsCleanParameters( request ) + || !SecurityUtil.isInternalRedirectUrlSafe( strForwardUrl, request ) ) { return AppPathService.getBaseUrl( request ); } diff --git a/src/java/fr/paris/lutece/util/http/SecurityUtil.java b/src/java/fr/paris/lutece/util/http/SecurityUtil.java index 080a8e3797..c679af68c5 100644 --- a/src/java/fr/paris/lutece/util/http/SecurityUtil.java +++ b/src/java/fr/paris/lutece/util/http/SecurityUtil.java @@ -33,6 +33,8 @@ */ package fr.paris.lutece.util.http; +import fr.paris.lutece.portal.service.util.AppPathService; +import fr.paris.lutece.portal.service.util.AppPropertiesService; import fr.paris.lutece.portal.web.LocalVariables; import fr.paris.lutece.util.string.StringUtil; @@ -43,6 +45,7 @@ import java.util.Enumeration; import javax.servlet.http.HttpServletRequest; +import org.springframework.util.AntPathMatcher; /** * Security utils @@ -61,6 +64,8 @@ public final class SecurityUtil "..", "/", "\\" }; + public static final String PROPERTY_REDIRECT_URL_SAFE_PATTERNS = "lutece.security.redirectUrlSafePatterns"; + // private static final String PATTERN_CLEAN_PARAMETER = "^[\\w/]+$+"; /** @@ -266,6 +271,82 @@ public static String getRealIp( HttpServletRequest request ) return strIPAddress; } + + /** + * Validate a forward URL to avoid open redirect + * with url safe patterns found in properties + * + * @see SecurityUtil#isInternalRedirectUrlSafe(java.lang.String, javax.servlet.http.HttpServletRequest, java.lang.String) + * + * @param strUrl + * @param request + * @return true if valid + */ + public static boolean isInternalRedirectUrlSafe( String strUrl, HttpServletRequest request ) + { + String strAntPathMatcherPatternsValues = AppPropertiesService.getProperty( SecurityUtil.PROPERTY_REDIRECT_URL_SAFE_PATTERNS ); + + return isInternalRedirectUrlSafe( strUrl, request, strAntPathMatcherPatternsValues ); + } + + + /** + * Validate an internal redirect URL to avoid internal open redirect. + * (Use this function only if the use of internal url redirect keys is not possible. + * For external url redirection control, use the plugin plugin-verifybackurl) + * + * the url should : + * - not be blank (null or empty string or spaces) + * - not start with "http://" or "https://" or "//" OR match the base URL or any URL in the pattern list + * + * example with a base url "https://lutece.fr/ : + * - valid : myapp/jsp/site/Portal.jsp , Another.jsp , https://lutece.fr/myapp/jsp/site/Portal.jsp + * - invalid : http://anothersite.com , https://anothersite.com , //anothersite.com , file://my.txt , ... + * + * + * @param strUrl the Url to validate + * @param request the current request (containing the baseUrl) + * @param strAntPathMatcherPatterns a comma separated list of AntPathMatcher patterns, as "http://**.lutece.com,https://**.lutece.com" + * @return true if valid + */ + public static boolean isInternalRedirectUrlSafe( String strUrl, HttpServletRequest request, String strAntPathMatcherPatterns ) + { + + if ( StringUtils.isBlank( strUrl ) ) return true ; // this is not a valid redirect Url, but it is not unsafe + + // filter schemes + if ( !strUrl.startsWith( "//" ) + && !strUrl.startsWith("http:" ) + && !strUrl.startsWith("https:" ) + && !strUrl.contains( "://" ) + && !strUrl.startsWith("javascript:" ) ) + return true ; // should be a relative path + + // compare with current baseUrl + if ( strUrl.startsWith( AppPathService.getBaseUrl( request ) ) ) + return true ; + + // compare with allowed url patterns + if ( !StringUtils.isBlank( strAntPathMatcherPatterns ) ) + { + AntPathMatcher pathMatcher = new AntPathMatcher(); + + String[] strAntPathMatcherPatternsTab = strAntPathMatcherPatterns.split( CONSTANT_COMMA ) ; + for ( String pattern : strAntPathMatcherPatternsTab ) + { + if ( pattern != null && pathMatcher.match( pattern , strUrl ) ) + return true ; + } + } + + + // the Url does not match the allowed patterns + Logger logger = Logger.getLogger( LOGGER_NAME ); + logger.warn( "SECURITY WARNING : OPEN_REDIRECT DETECTED : " + dumpRequest( request ) ); + + return false; + + } /** * Identify user data saved in log files to prevent Log Forging attacks diff --git a/src/test/java/fr/paris/lutece/util/http/SecurityUtilTest.java b/src/test/java/fr/paris/lutece/util/http/SecurityUtilTest.java index 219886df1b..0cfc00d97b 100644 --- a/src/test/java/fr/paris/lutece/util/http/SecurityUtilTest.java +++ b/src/test/java/fr/paris/lutece/util/http/SecurityUtilTest.java @@ -84,4 +84,67 @@ public void testContainsCleanParameters( ) request.setParameter( "param4", "}" ); assertTrue( SecurityUtil.containsCleanParameters( request ) ); } + + + /** + * Test of isRedirectUrlSafe method, of class SecurityUtil, to avoid open redirect + */ + @Test + public void testIsRedirectUrlSafe( ) + { + System.out.println( "isRedirectUrlSafe" ); + + MockHttpServletRequest request = new MockHttpServletRequest( ); + + // Assert False + String strUrl = "http://anothersite.com"; + request.setParameter( "url", strUrl ); + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "//anothersite.com"; + request.setParameter( "url", strUrl ); + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "file://my.txt"; + request.setParameter( "url", strUrl ); + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "javascript:alert('hello');"; + request.setParameter( "url", strUrl ); + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "opera-http://anothersite.com"; + request.setParameter( "url", strUrl ); + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "http://another.subdomain.mylutece.com"; + request.setParameter( "url", strUrl ); + String strUrlPatterns="http://**.lutece.com,https://**.lutece.com"; + assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request, strUrlPatterns ) ); + + // Assert True + strUrl = null; + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request) ); + + strUrl = ""; + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request) ); + + strUrl = "/jsp/site/Portal.jsp"; + request.setParameter( "url", strUrl ); + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "Another.jsp"; + request.setParameter( "url", strUrl ); + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "http://localhost/myapp/jsp/site/Portal.jsp"; + request.setParameter( "url", strUrl ); + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) ); + + strUrl = "http://another.subdomain.lutece.com"; + request.setParameter( "url", strUrl ); + strUrlPatterns="http://**.lutece.com/**,https://**.lutece.com/**"; + assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request, strUrlPatterns ) ); + + } } diff --git a/webapp/WEB-INF/conf/lutece.properties b/webapp/WEB-INF/conf/lutece.properties index 7ec593576b..9b645c7e28 100644 --- a/webapp/WEB-INF/conf/lutece.properties +++ b/webapp/WEB-INF/conf/lutece.properties @@ -227,6 +227,15 @@ askPasswordReinitialization.admin.level=0 input.xss.characters=<>#" xss.error.message= Les caract\u00e8res < > # & et " sont interdits dans le contenu de votre message. +################################################################################ +# Internal Redirect URL safe patterns +# +# The list of "safe" AntPathMatcher patterns used by SecurityUtil.isReturnURLSafe function +# to avoid Open Redirect attacks. +# This should be a comma separated list of AntPathMatcher patterns, +# ex: +#lutece.security.redirectUrlSafePatterns=http://**.lutece.com/**,https://**.lutece.com/** + ################################################################################ # Paginators #