JSPWIKI-1138 - Fix XSS vulnerability in property handling.

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_

jspwiki-api/src/main/java/org/apache/wiki/api/Release.java

@@ -69,7 +69,7 @@ public final class Release {
      *  <p>
      *  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

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.
+     * <p>
+     * This method performs a two-step process on the property value:
+     * <ol>
+     *     <li><b>Sanitization:</b> 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 "&lt;", "&gt;", "&quot;",
+     *         "&#x27;", and "&#x2F;", respectively.</li>
+     *     <li><b>Normalization:</b> 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.</li>
+     * </ol>
+     * If the property key is null or the property value is not set, the method performs no action.
+     * </p>
      *
-     * @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 "&lt;", "&gt;", "&quot;", "&#x27;", and "&#x2F;", 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( "<", "&lt;" )
+                .replaceAll( ">", "&gt;" )
+                .replaceAll( "\"", "&quot;" )
+                .replaceAll( "'", "&#x27;" )
+                .replaceAll( "/", "&#x2F;" ) );
+    }
 
 }

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", "<script>alert('xss');</script>" );
+        req.setParameter( "jspwiki.workDir", "C:\\Users\\Example\\Path" );
+        req.setParameter( "jspwiki.fileSystemProvider.pageDir", "<script>alert2('xss');</script>" );
+
+
+        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( "&lt;script&gt;alert(&#x27;xss&#x27;);&lt;&#x2F;script&gt;", 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( "&lt;script&gt;alert2(&#x27;xss&#x27;);&lt;&#x2F;script&gt;", 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 );
+        }
+    }
+
+}
+