Skip to content

Commit bdf1133

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

File tree

3 files changed

+80
-1
lines changed

3 files changed

+80
-1
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ public String doModifyUserTheme( HttpServletRequest request, HttpServletResponse
138138
String strTheme = request.getParameter( PARAMETER_THEME );
139139
String strForwardUrl = request.getParameter( PARAMETER_URL );
140140

141-
if ( !SecurityUtil.containsCleanParameters( request ) )
141+
if ( !SecurityUtil.containsCleanParameters( request )
142+
|| !SecurityUtil.isForwardUrlValid( strForwardUrl, request) )
142143
{
143144
return AppPathService.getBaseUrl( request );
144145
}

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

+35
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

@@ -266,6 +267,40 @@ public static String getRealIp( HttpServletRequest request )
266267

267268
return strIPAddress;
268269
}
270+
271+
/**
272+
* Validate a forward URL to avoid open redirect
273+
* the url should :
274+
* - not be blank (null or empty string or spaces)
275+
* - start with the base URL, or not start with "http" or "//"
276+
*
277+
* example with a base url "https://lutece.fr/ :
278+
* - valid : /jsp/site/Portal.jsp , Another.jsp , https://lutece.fr/jsp/site/Portal.jsp
279+
* - invalid : http://anothersite.com , https://anothersite.com , //anothersite.com , file://my.txt , ...
280+
*
281+
* @param strForwardUrl
282+
* @param request
283+
* @return true if valid
284+
*/
285+
public static boolean isForwardUrlValid( String strForwardUrl, HttpServletRequest request )
286+
{
287+
288+
if ( StringUtils.isBlank( strForwardUrl) ) return false ;
289+
290+
if ( ( strForwardUrl.startsWith( "//" ) || strForwardUrl.contains( "://" ) )
291+
&& !strForwardUrl.startsWith ( AppPathService.getBaseUrl( request ) ) )
292+
{
293+
Logger logger = Logger.getLogger( LOGGER_NAME );
294+
logger.warn( "SECURITY WARNING : OPEN_REDIRECT DETECTED : " + dumpRequest( request ) );
295+
296+
return false;
297+
}
298+
else
299+
{
300+
return true;
301+
}
302+
}
303+
269304

270305
/**
271306
* Identify user data saved in log files to prevent Log Forging attacks

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

+43
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,47 @@ public void testContainsCleanParameters( )
8484
request.setParameter( "param4", "}" );
8585
assertTrue( SecurityUtil.containsCleanParameters( request ) );
8686
}
87+
88+
89+
/**
90+
* Test of isForwardUrlValid method, of class SecurityUtil, to avoid open redirect
91+
*/
92+
@Test
93+
public void testisForwardUrlValid( )
94+
{
95+
System.out.println( "isForwardUrlValid" );
96+
97+
MockHttpServletRequest request = new MockHttpServletRequest( );
98+
99+
String strForwardUrl = null;
100+
assertFalse( SecurityUtil.isForwardUrlValid( strForwardUrl, request) );
101+
102+
strForwardUrl = "";
103+
request.setParameter( "url", strForwardUrl );
104+
assertFalse( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
105+
106+
strForwardUrl = "http://anothersite.com";
107+
request.setParameter( "url", strForwardUrl );
108+
assertFalse( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
109+
110+
strForwardUrl = "//anothersite.com";
111+
request.setParameter( "url", strForwardUrl );
112+
assertFalse( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
113+
114+
strForwardUrl = "file://my.txt";
115+
request.setParameter( "url", strForwardUrl );
116+
assertFalse( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
117+
118+
strForwardUrl = "/jsp/site/Portal.jsp";
119+
request.setParameter( "url", strForwardUrl );
120+
assertTrue( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
121+
122+
strForwardUrl = "Another.jsp";
123+
request.setParameter( "url", strForwardUrl );
124+
assertTrue( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
125+
126+
strForwardUrl = "http://localhost/myapp/jsp/site/Portal.jsp";
127+
request.setParameter( "url", strForwardUrl );
128+
assertTrue( SecurityUtil.isForwardUrlValid( strForwardUrl, request ) );
129+
}
87130
}

0 commit comments

Comments
 (0)