Skip to content

Commit ba3b6f4

Browse files
committed
LUTECE-2206 : Avoid open redirect when modifying a theme
1 parent 342ca3e commit ba3b6f4

File tree

3 files changed

+126
-1
lines changed

3 files changed

+126
-1
lines changed

src/java/fr/paris/lutece/portal/web/style/ThemesJspBean.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import fr.paris.lutece.portal.service.portal.ThemesService;
4141
import fr.paris.lutece.portal.service.template.AppTemplateService;
4242
import fr.paris.lutece.portal.service.util.AppPathService;
43+
import fr.paris.lutece.portal.service.util.AppPropertiesService;
4344
import fr.paris.lutece.portal.web.admin.AdminFeaturesPageJspBean;
4445
import fr.paris.lutece.portal.web.constants.Messages;
4546
import fr.paris.lutece.util.html.HtmlTemplate;
@@ -137,8 +138,10 @@ public String doModifyUserTheme( HttpServletRequest request, HttpServletResponse
137138
{
138139
String strTheme = request.getParameter( PARAMETER_THEME );
139140
String strForwardUrl = request.getParameter( PARAMETER_URL );
141+
String strAntPathMatcherPatternsValues = AppPropertiesService.getProperty( SecurityUtil.PROPERTY_ANTPATHMATCHER_PATTERNS );
140142

141-
if ( !SecurityUtil.containsCleanParameters( request ) )
143+
if ( !SecurityUtil.containsCleanParameters( request )
144+
|| !SecurityUtil.isRedirectUrlSafe( strForwardUrl, request, strAntPathMatcherPatternsValues) )
142145
{
143146
return AppPathService.getBaseUrl( request );
144147
}

src/java/fr/paris/lutece/util/http/SecurityUtil.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
*/
3434
package fr.paris.lutece.util.http;
3535

36+
import fr.paris.lutece.portal.service.util.AppPathService;
3637
import fr.paris.lutece.portal.web.LocalVariables;
3738
import fr.paris.lutece.util.string.StringUtil;
3839

@@ -43,6 +44,7 @@
4344
import java.util.Enumeration;
4445

4546
import javax.servlet.http.HttpServletRequest;
47+
import org.springframework.util.AntPathMatcher;
4648

4749
/**
4850
* Security utils
@@ -54,13 +56,16 @@ public final class SecurityUtil
5456
private static final String CONSTANT_HTTP_HEADER_X_FORWARDED_FOR = "X-Forwarded-For";
5557
private static final String PATTERN_IP_ADDRESS = "^([0-9]{1,3}\\.){3}[0-9]{1,3}$";
5658
private static final String CONSTANT_COMMA = ",";
59+
private static final String CONSTANT_PATH_SEPARATOR = "." ;
5760
private static final String [ ] XXE_TERMS = {
5861
"!DOCTYPE", "!ELEMENT", "!ENTITY"
5962
};
6063
private static final String [ ] PATH_MANIPULATION = {
6164
"..", "/", "\\"
6265
};
6366

67+
public static final String PROPERTY_ANTPATHMATCHER_PATTERNS = "lutece.security.antpathmatcher.patterns";
68+
6469
// private static final String PATTERN_CLEAN_PARAMETER = "^[\\w/]+$+";
6570

6671
/**
@@ -266,6 +271,75 @@ public static String getRealIp( HttpServletRequest request )
266271

267272
return strIPAddress;
268273
}
274+
275+
/**
276+
* Validate a forward URL to avoid open redirect
277+
* [see isRedirectUrlSafe(String, HttpServletRequest, String)]
278+
*
279+
* @param strUrl
280+
* @param request
281+
* @return true if valid
282+
*/
283+
public static boolean isRedirectUrlSafe( String strUrl, HttpServletRequest request )
284+
{
285+
return isRedirectUrlSafe( strUrl, request, null );
286+
}
287+
288+
289+
/**
290+
* Validate a forward URL to avoid open redirect
291+
* the url should :
292+
* - not be blank (null or empty string or spaces)
293+
* - not start with "http://" or "https://" or "//" OR match the base URL or any URL in the pattern list
294+
*
295+
* example with a base url "https://lutece.fr/ :
296+
* - valid : myapp/jsp/site/Portal.jsp , Another.jsp , https://lutece.fr/myapp/jsp/site/Portal.jsp
297+
* - invalid : http://anothersite.com , https://anothersite.com , //anothersite.com , file://my.txt , ...
298+
*
299+
*
300+
* @param strUrl the Url to validate
301+
* @param request the current request (containing the baseUrl)
302+
* @param strAntPathMatcherPatterns a comma separated list of AntPathMatcher patterns, as "http://**.lutece.com,https://**.lutece.com"
303+
* @return true if valid
304+
*/
305+
public static boolean isRedirectUrlSafe( String strUrl, HttpServletRequest request, String strAntPathMatcherPatterns )
306+
{
307+
308+
if ( StringUtils.isBlank( strUrl) ) return false ;
309+
AntPathMatcher pathMatcher = new AntPathMatcher();
310+
pathMatcher.setPathSeparator( CONSTANT_PATH_SEPARATOR );
311+
312+
if ( !strUrl.startsWith( "//" ) && !strUrl.startsWith("http://" ) && !strUrl.startsWith("https://" ) && !strUrl.startsWith("javascript:" ) )
313+
{
314+
// relative path
315+
return true ;
316+
}
317+
else
318+
{
319+
// check base urls
320+
if ( !pathMatcher.matchStart( strUrl, AppPathService.getBaseUrl( request ) ) )
321+
return true ;
322+
323+
if ( strAntPathMatcherPatterns != null )
324+
{
325+
String[] strAntPathMatcherPatternsTab = strAntPathMatcherPatterns.split( CONSTANT_COMMA ) ;
326+
for ( String pattern : strAntPathMatcherPatternsTab )
327+
{
328+
if ( pattern != null && pathMatcher.match( pattern , strUrl ) )
329+
return true ;
330+
}
331+
}
332+
333+
334+
// the Url does not match the allowed patterns
335+
Logger logger = Logger.getLogger( LOGGER_NAME );
336+
logger.warn( "SECURITY WARNING : OPEN_REDIRECT DETECTED : " + dumpRequest( request ) );
337+
338+
return false;
339+
}
340+
341+
}
342+
269343

270344
/**
271345
* Identify user data saved in log files to prevent Log Forging attacks

src/test/java/fr/paris/lutece/util/http/SecurityUtilTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,52 @@ public void testContainsCleanParameters( )
8484
request.setParameter( "param4", "}" );
8585
assertTrue( SecurityUtil.containsCleanParameters( request ) );
8686
}
87+
88+
89+
/**
90+
* Test of isRedirectUrlSafe method, of class SecurityUtil, to avoid open redirect
91+
*/
92+
@Test
93+
public void testIsRedirectUrlSafe( )
94+
{
95+
System.out.println( "isRedirectUrlSafe" );
96+
97+
MockHttpServletRequest request = new MockHttpServletRequest( );
98+
99+
String strUrl = null;
100+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request) );
101+
102+
strUrl = "";
103+
request.setParameter( "url", strUrl );
104+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
105+
106+
strUrl = "http://anothersite.com";
107+
request.setParameter( "url", strUrl );
108+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
109+
110+
strUrl = "//anothersite.com";
111+
request.setParameter( "url", strUrl );
112+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
113+
114+
strUrl = "file://my.txt";
115+
request.setParameter( "url", strUrl );
116+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
117+
118+
strUrl = "javascript:alert('hello');";
119+
request.setParameter( "url", strUrl );
120+
assertFalse( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
121+
122+
strUrl = "/jsp/site/Portal.jsp";
123+
request.setParameter( "url", strUrl );
124+
assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
125+
126+
strUrl = "Another.jsp";
127+
request.setParameter( "url", strUrl );
128+
assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
129+
130+
strUrl = "http://localhost/myapp/jsp/site/Portal.jsp";
131+
request.setParameter( "url", strUrl );
132+
assertTrue( SecurityUtil.isRedirectUrlSafe( strUrl, request ) );
133+
134+
}
87135
}

0 commit comments

Comments
 (0)