Patchwork PR: Autofix

dubbo-admin/src/main/java/com/alibaba/dubbo/governance/web/common/interceptor/AuthorizationValve.java

@@ -25,9 +25,7 @@
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.UUID;
+import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -40,6 +38,11 @@
 public class AuthorizationValve extends AbstractValve {
 
     private static final Logger logger = LoggerFactory.getLogger(AuthorizationValve.class);
+    private static final Set<String> SAFE_REDIRECT_PATHS = new HashSet<>(Arrays.asList(
+        "/",
+        "/login",
+        "/dashboard"
+    ));
     private static final String BASIC_CHALLENGE = "Basic";
     private static final String DIGEST_CHALLENGE = "Digest";
     private static final String CHALLENGE = BASIC_CHALLENGE;
@@ -71,6 +74,16 @@ static byte[] readToBytes(InputStream in) throws IOException {
         return buf;
     }
 
+    private void validateAndRedirect(HttpServletResponse response, String redirectPath) throws IOException {
+        if (SAFE_REDIRECT_PATHS.contains(redirectPath)) {
+            response.sendRedirect(redirectPath);
+        } else {
+            // Default to a safe path
+            response.sendRedirect("/");
+            logger.warn("Attempted redirect to unauthorized path: " + redirectPath);
+        }
+    }
+
     @Override
     protected void init() throws Exception {
     }
@@ -90,7 +103,7 @@ public void invoke(PipelineContext pipelineContext) throws Exception {
                 showLoginForm();
             } else {
                 setLogout(false);
-                response.sendRedirect(contextPath == null || contextPath.length() == 0 ? "/" : contextPath);
+                validateAndRedirect(response, contextPath == null || contextPath.length() == 0 ? "/" : contextPath);
             }
             return;
         }

dubbo-admin/src/main/java/com/alibaba/dubbo/governance/web/governance/module/screen/Providers.java

@@ -142,9 +142,12 @@ private void setSearchHistroy(Map<String, Object> context, String value) {
         }
 
         Cookie _cookie = new Cookie("HISTORY", newCookiev);
-        _cookie.setMaxAge(60 * 60 * 24 * 7); // 设置Cookie的存活时间为30分钟
+        _cookie.setMaxAge(60 * 60 * 24 * 7);  // 7 days lifetime
         _cookie.setPath("/");
-        response.addCookie(_cookie); // 写入客户端硬盘
+        _cookie.setSecure(true);              // Only sent over HTTPS
+        _cookie.setHttpOnly(true);            // Prevent client-side access
+        response.setHeader("Set-Cookie", _cookie.getName() + "=" + _cookie.getValue() + "; SameSite=Strict");          // Protect against CSRF
+        response.addCookie(_cookie);
     }
 
     public void show(Long id, Map<String, Object> context) {

dubbo-admin/src/main/webapp/SpryAssets/SpryValidationRadio.js

@@ -188,15 +188,36 @@ Spry.Widget.ValidationRadio.prototype.getRadios = function () {
 };
 
 Spry.Widget.ValidationRadio.prototype.addClassName = function (ele, className) {
-    if (!ele || !className || (ele.className && ele.className.search(new RegExp("\\b" + className + "\\b")) != -1))
-        return;
-    ele.className += (ele.className ? " " : "") + className;
+    if (!ele || !className || !ele.className) return;
+
+    // Validate className to ensure it only contains safe characters
+    if (!/^[a-zA-Z0-9_-]+$/.test(className)) return;
+
+    // Only add if not already present
+    const classes = ele.className.split(' ');
+    if (!classes.includes(className)) {
+        classes.push(className);
+        ele.className = classes.join(' ');
+    }
+};
+    // Only add if not already present
+    const classes = ele.className.split(' ');
+    if (!classes.includes(className)) {
+        classes.push(className);
+        ele.className = classes.join(' ');
+    }
 };
 
 Spry.Widget.ValidationRadio.prototype.removeClassName = function (ele, className) {
-    if (!ele || !className || (ele.className && ele.className.search(new RegExp("\\b" + className + "\\b")) == -1))
-        return;
-    ele.className = ele.className.replace(new RegExp("\\s*\\b" + className + "\\b", "g"), "");
+    if (!ele || !className || !ele.className) return;
+
+    // Validate className to ensure it only contains safe characters
+    if (!/^[a-zA-Z0-9_-]+$/.test(className)) return;
+
+    // Use a predefined pattern or string-based replacement
+    ele.className = ele.className.split(' ')
+        .filter(cls => cls !== className)
+        .join(' ');
 };
 
 Spry.Widget.ValidationRadio.prototype.onFocus = function (e) {

test_fix.js

@@ -0,0 +1,96 @@
+// Direct test of the fixed className manipulation functions
+function testClassOperations() {
+    console.log('Starting tests...');
+
+    // Create the prototype functions we modified
+    const proto = {
+        addClassName: function (ele, className) {
+            if (!ele || !className || !ele.className) return;
+
+            // Validate className to ensure it only contains safe characters
+            if (!/^[a-zA-Z0-9_-]+$/.test(className)) return;
+
+            // Only add if not already present
+            const classes = ele.className.split(' ');
+            if (!classes.includes(className)) {
+                classes.push(className);
+                ele.className = classes.join(' ');
+            }
+        },
+
+        removeClassName: function (ele, className) {
+            if (!ele || !className || !ele.className) return;
+
+            // Validate className to ensure it only contains safe characters
+            if (!/^[a-zA-Z0-9_-]+$/.test(className)) return;
+
+            // Use a predefined pattern or string-based replacement
+            ele.className = ele.className.split(' ')
+                .filter(cls => cls !== className)
+                .join(' ');
+        }
+    };
+
+    // Test 1: Normal operation
+    console.log('\nTest 1: Normal operation');
+    const element1 = { className: 'initial-class' };
+    proto.addClassName(element1, 'test-class');
+    console.log('After add:', element1.className);
+    proto.removeClassName(element1, 'test-class');
+    console.log('After remove:', element1.className);
+
+    // Test 2: Malicious input that would have caused ReDoS
+    console.log('\nTest 2: Malicious input');
+    const element2 = { className: 'safe-class' };
+    const maliciousClassName = 'x' + '?'.repeat(100) + '{1}' + 'y';
+    console.time('Malicious operation');
+    proto.addClassName(element2, maliciousClassName);
+    proto.removeClassName(element2, maliciousClassName);
+    console.timeEnd('Malicious operation');
+    console.log('Class after malicious attempt:', element2.className, '(should remain unchanged)');
+
+    // Test 3: Multiple classes
+    console.log('\nTest 3: Multiple classes');
+    const element3 = { className: 'class1 class2' };
+    proto.addClassName(element3, 'class3');
+    console.log('After adding third class:', element3.className);
+    proto.removeClassName(element3, 'class2');
+    console.log('After removing middle class:', element3.className);
+
+    // Test 4: Edge cases
+    console.log('\nTest 4: Edge cases');
+    const element4 = { className: 'starting-class' };
+
+    // Empty string
+    proto.addClassName(element4, '');
+    console.log('After adding empty string:', element4.className, '(should remain unchanged)');
+
+    // Special characters (should be rejected)
+    proto.addClassName(element4, 'invalid*class');
+    console.log('After attempting to add invalid class:', element4.className, '(should remain unchanged)');
+
+    // Null/undefined element
+    proto.addClassName(null, 'test');
+    proto.removeClassName(undefined, 'test');
+    console.log('Survived null/undefined tests');
+
+    // Test 5: Valid class names with different patterns
+    console.log('\nTest 5: Valid class names');
+    const element5 = { className: 'base' };
+    const validClasses = ['simple', 'with-dash', 'with_underscore', 'alphaNum123'];
+
+    validClasses.forEach(cls => {
+        proto.addClassName(element5, cls);
+        console.log(`Added ${cls}:`, element5.className);
+    });
+
+    // Test 6: Duplicate prevention
+    console.log('\nTest 6: Duplicate prevention');
+    const element6 = { className: 'test' };
+    proto.addClassName(element6, 'test');
+    console.log('After adding duplicate class:', element6.className, '(should not duplicate)');
+
+    console.log('\nAll tests completed.');
+}
+
+testClassOperations();

test_redos.js

@@ -0,0 +1,16 @@
+const className = "valid"; // Normal case
+const maliciousClassName = "(a+)+" + "a".repeat(100); // Malicious input with catastrophic backtracking
+
+function testRegExpPerformance(input) {
+    console.time('RegExp Test');
+    const regex = new RegExp("\\s|^" + input + "\\s|$");
+    const testStr = "a".repeat(100);
+    regex.test(testStr);
+    console.timeEnd('RegExp Test');
+}
+
+console.log("Testing with normal input:");
+testRegExpPerformance(className);
+
+console.log("\nTesting with malicious input:");
+testRegExpPerformance(maliciousClassName);