diff --git a/ChangeLog.md b/ChangeLog.md index 4c8c882d0d..79e76b393f 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -17,6 +17,12 @@ specific language governing permissions and limitations under the License. --> +**2023-11-26 Arturo Bernal (juanpablo AT apache DOT org)** + +* _2.12.2-git-10_ + +* [JSPWIKI-1138](https://issues.apache.org/jira/browse/JSPWIKI-1138) Fix XSS vulnerability in property handling + **2023-11-25 Juan Pablo Santos (juanpablo AT apache DOT org)** * _2.12.2-git-09_ diff --git a/jspwiki-api/src/main/java/org/apache/wiki/api/Release.java b/jspwiki-api/src/main/java/org/apache/wiki/api/Release.java index 97c8502516..ff567b0493 100644 --- a/jspwiki-api/src/main/java/org/apache/wiki/api/Release.java +++ b/jspwiki-api/src/main/java/org/apache/wiki/api/Release.java @@ -69,7 +69,7 @@ public final class Release { *

* If the build identifier is empty, it is not added. */ - public static final String BUILD = "09"; + public static final String BUILD = "10"; /** * This is the generic version string you should use when printing out the version. It is of diff --git a/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java b/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java index 570d9722e0..28059a9b61 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java @@ -18,6 +18,7 @@ Licensed to the Apache Software Foundation (ASF) under one */ package org.apache.wiki.ui; +import org.apache.commons.io.FilenameUtils; import org.apache.wiki.api.core.Engine; import org.apache.wiki.api.core.Session; import org.apache.wiki.api.providers.AttachmentProvider; @@ -181,17 +182,18 @@ public void parseProperties () { // Get application name String nullValue = m_props.getProperty( APP_NAME, rb.getString( "install.installer.default.appname" ) ); - parseProperty( APP_NAME, nullValue ); + parseProperty( APP_NAME, nullValue ); + sanitizeInput( APP_NAME ); // Get/sanitize page directory nullValue = m_props.getProperty( PAGE_DIR, rb.getString( "install.installer.default.pagedir" ) ); parseProperty( PAGE_DIR, nullValue ); - sanitizePath( PAGE_DIR ); + sanitizeAndNormalizePath( PAGE_DIR ); // Get/sanitize work directory nullValue = m_props.getProperty( WORK_DIR, TMP_DIR ); parseProperty( WORK_DIR, nullValue ); - sanitizePath( WORK_DIR ); + sanitizeAndNormalizePath( WORK_DIR ); // Set a few more default properties, for easy setup m_props.setProperty( STORAGE_DIR, m_props.getProperty( PAGE_DIR ) ); @@ -239,17 +241,34 @@ private void parseProperty( final String param, final String defaultValue ) { } m_props.put( param, value ); } - + /** - * Simply sanitizes any path which contains backslashes (sometimes Windows users may have them) by expanding them to double-backslashes + * Sanitizes and normalizes the file path associated with the specified property key. + *

+ * This method performs a two-step process on the property value: + *

    + *
  1. Sanitization: It first sanitizes the input to mitigate the risk of Cross-Site Scripting (XSS) attacks. + * This is achieved by replacing specific characters known to be used in XSS attacks with their corresponding + * HTML entity codes. The characters '<', '>', '"', ''', and '/' are replaced with "<", ">", """, + * "'", and "/", respectively.
    2. + *
  2. Normalization: After sanitization, the method normalizes the file path using + * {@link org.apache.commons.io.FilenameUtils#normalizeNoEndSeparator(String)}. This normalization process + * ensures that the file path is in a consistent format, which is particularly important for file system + * operations, especially in environments like Windows where backslashes are used.
    3. + *
+ * If the property key is null or the property value is not set, the method performs no action. + *

* - * @param key the key of the property to sanitize + * @param key The key of the property whose value represents a file path. This key is used to retrieve, + * sanitize, and normalize the property value. If the key is null, no action is taken. */ - private void sanitizePath( final String key ) { - String s = m_props.getProperty( key ); - s = TextUtil.replaceString(s, "\\", "\\\\" ); - s = s.trim(); - m_props.put( key, s ); + private void sanitizeAndNormalizePath( final String key ) { + sanitizeInput( key ); // Sanitize the input for XSS protection + final String sanitizedValue = m_props.getProperty( key ); + if( sanitizedValue != null ) { + final String normalizedPath = FilenameUtils.normalizeNoEndSeparator( sanitizedValue ); + m_props.put( key, normalizedPath ); + } } private void validateNotNull( final String key, final String message ) { @@ -258,5 +277,22 @@ private void validateNotNull( final String key, final String message ) { m_session.addMessage( INSTALL_ERROR, message ); } } + + /** + * Sanitizes the property value associated with the specified key to prevent Cross-Site Scripting (XSS) attacks. + * This method retrieves the property value for the given key from the properties map, then replaces certain + * characters with their corresponding HTML entity codes to mitigate the risk of XSS vulnerabilities. + * The characters '<', '>', '"', ''', and '/' are replaced with "<", ">", """, "'", and "/", respectively. + * If the key is null, the method returns without making any changes. + * + * @param key The key of the property to be sanitized. If the key is null, no action is taken. + */ + private void sanitizeInput( final String key ) { + m_props.put( key, m_props.getProperty( key ).replaceAll( "<", "<" ) + .replaceAll( ">", ">" ) + .replaceAll( "\"", """ ) + .replaceAll( "'", "'" ) + .replaceAll( "/", "/" ) ); + } } diff --git a/jspwiki-main/src/test/java/org/apache/wiki/ui/InstallerTest.java b/jspwiki-main/src/test/java/org/apache/wiki/ui/InstallerTest.java new file mode 100644 index 0000000000..8e99d1f4dc --- /dev/null +++ b/jspwiki-main/src/test/java/org/apache/wiki/ui/InstallerTest.java @@ -0,0 +1,112 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + */ +/* + * (C) Janne Jalkanen 2005 + * + */ + +package org.apache.wiki.ui; + +import net.sourceforge.stripes.mock.MockHttpServletRequest; +import net.sourceforge.stripes.mock.MockServletConfig; +import org.apache.wiki.TestEngine; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.servlet.ServletException; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; + + +public class InstallerTest { + + private Installer installer; + + + @BeforeEach + public void setUp() throws ServletException { + + + final IntallerMockHttpServletRequest req = new IntallerMockHttpServletRequest( "/JSPWiki", "/wiki/Wiki.jsp" ); + + + req.setParameter( "jspwiki.applicationName", "" ); + req.setParameter( "jspwiki.workDir", "C:\\Users\\Example\\Path" ); + req.setParameter( "jspwiki.fileSystemProvider.pageDir", "" ); + + + final MockServletConfig config = new MockServletConfig(); + config.setServletContext( TestEngine.createServletContext( "/JSPWiki" ) ); + + + installer = new Installer( req, config ); + } + + @Test + public void testSanitizeInput() { + // Assuming there's a method in Installer that processes the request and sets properties + installer.parseProperties(); + + // Verify the property is sanitized + String sanitizedValue = installer.getProperty( "jspwiki.applicationName" ); + assertEquals( "<script>alert('xss');</script>", sanitizedValue ); + } + + @Test + public void testSanitizePath() { + // Assuming there's a method in Installer that processes the request and sets properties + installer.parseProperties(); + + // Verify the path is normalized + String normalizedPath = installer.getProperty( "jspwiki.workDir" ); + assertEquals( "C:/Users/Example/Path", normalizedPath ); // Adjust expected value based on normalization behavior + } + + @Test + public void testSanitizePageDir() { + // Assuming there's a method in Installer that processes the request and sets properties + installer.parseProperties(); + + // Verify the path is normalized + String sanitizedValue = installer.getProperty( "jspwiki.fileSystemProvider.pageDir" ); + assertEquals( "<script>alert2('xss');</script>", sanitizedValue ); + } + + + static class IntallerMockHttpServletRequest extends MockHttpServletRequest { + private final Map< String, String > parameters = new HashMap<>(); + + public IntallerMockHttpServletRequest( String contextPath, String servletPath ) { + super( contextPath, servletPath ); + } + + @Override + public String getParameter( String name ) { + return parameters.get( name ); + } + + public void setParameter( String name, String value ) { + parameters.put( name, value ); + } + } + +} +