LUTECE-2206 : Avoid open redirect when modifying a theme

Author: seboo

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.isRedirectUrlSafe( strForwardUrl, request ) )
         {
             return AppPathService.getBaseUrl( request );
         }

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,79 @@ public static String getRealIp( HttpServletRequest request )
 
         return strIPAddress;
     }
+    
+    /**
+     * Validate a forward URL to avoid open redirect
+     * [see isRedirectUrlSafe(String, HttpServletRequest, String)]
+     * 
+     * @param strUrl
+     * @param request
+     * @return true if valid
+     */
+    public static boolean isRedirectUrlSafe( String strUrl, HttpServletRequest request )
+    {
+        String strAntPathMatcherPatternsValues = AppPropertiesService.getProperty( SecurityUtil.PROPERTY_REDIRECT_URL_SAFE_PATTERNS );
+
+        return isRedirectUrlSafe( strUrl, request, strAntPathMatcherPatternsValues );
+    }
+
+
+    /**
+     * Validate a redirect URL to avoid open redirect.
+     * (Use this function only if the use of url redirect keys is not possible)
+     * 
+     * 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 isRedirectUrlSafe( 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

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 ) );
+
+    }
 }

webapp/WEB-INF/conf/lutece.properties

@@ -227,6 +227,14 @@ askPasswordReinitialization.admin.level=0
 input.xss.characters=<>#"
 xss.error.message= Les caract\u00e8res &lt; &gt; # &amp; et &quot; sont interdits dans le contenu de votre message.
 
+################################################################################
+# 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: "http://**.lutece.com/**,https://**.lutece.com/**"
+lutece.security.redirectUrlSafePatterns=
+
 ################################################################################
 # Paginators
 #