fix(security): restore outgoing message integrity, add HSTS/Permissions-Policy

- Remove send-path keyword blocklist that silently rejected legitimate messages (e.g. "constructor", "global", "document.", literal "javascript:") without adding protection. XSS is enforced at the rendering boundary by the receive-side DOMPurify pass and by sanitizeMessage() before encryption. - Preserve newlines/tabs/indentation in _sanitizeInputString; stop collapsing all whitespace which destroyed multi-line messages and code snippets. - Stop logging raw AAD (sessionId + keyFingerprint) on validation failure; log length only, in both message and file-message AAD validators. - Add Strict-Transport-Security (2y + preload) and Permissions-Policy (camera=self for QR, rest denied) to nginx.conf and .htaccess. - Add tests/outgoing-message-integrity.test.mjs regression suite.
2026-06-18 16:48:29 -04:00
parent 6f36fce8c6
commit 42be55aaeb
7 changed files with 155 additions and 139 deletions
@@ -143,6 +143,10 @@
    Header set X-Content-Type-Options "nosniff"
    Header set Referrer-Policy "strict-origin-when-cross-origin"
    Header set X-Frame-Options "DENY"
+    # Force HTTPS (2 years + preload) to close the first-visit SSL-strip window.
+    Header set Strict-Transport-Security "max-age=63072000; includeSubDomains; preload"
+    # Restrict powerful features; camera kept for in-page QR scanning.
+    Header set Permissions-Policy "camera=(self), microphone=(), geolocation=(), payment=(), usb=(), magnetometer=(), gyroscope=(), accelerometer=()"
 </IfModule>

 # Content Security Policy (frame-ancestors and report-uri only work in HTTP headers, not meta tags)
@@ -55,6 +55,12 @@ http {
        add_header Referrer-Policy          "strict-origin-when-cross-origin"  always;
        add_header X-Frame-Options          "DENY"                             always;
        add_header Content-Security-Policy   "frame-ancestors 'none';"         always;
+        # Force HTTPS for two years and preload, closing the first-visit SSL-strip
+        # window that upgrade-insecure-requests alone does not cover.
+        add_header Strict-Transport-Security "max-age=63072000; includeSubDomains; preload" always;
+        # Lock down powerful features. Camera is allowed for in-page QR scanning;
+        # microphone/geolocation and other sensors are denied outright.
+        add_header Permissions-Policy        "camera=(self), microphone=(), geolocation=(), payment=(), usb=(), magnetometer=(), gyroscope=(), accelerometer=()" always;
        add_header Cache-Control            $sb_cache                          always;
        add_header Service-Worker-Allowed   "/"                                always;

@@ -7183,17 +7183,6 @@ var EnhancedSecureWebRTCManager = class _EnhancedSecureWebRTCManager {
          validationResult.errors.push(`String too long: ${data.length} > ${this._inputValidationLimits.maxStringLength}`);
          return validationResult;
        }
-        for (const pattern of this._maliciousPatterns) {
-          if (pattern.test(data)) {
-            validationResult.errors.push(`Malicious pattern detected: ${pattern.source}`);
-            this._secureLog("warn", "\u{1F6A8} Malicious pattern detected in input", {
-              context,
-              pattern: pattern.source,
-              dataLength: data.length
-            });
-            return validationResult;
-          }
-        }
        validationResult.sanitizedData = this._sanitizeInputString(data);
        validationResult.isValid = true;
        return validationResult;
@@ -7275,8 +7264,9 @@ var EnhancedSecureWebRTCManager = class _EnhancedSecureWebRTCManager {
   */
  _sanitizeInputString(str) {
    if (typeof str !== "string") return str;
-    str = str.replace(/\0/g, "");
-    str = str.replace(/\s+/g, " ");
+    str = str.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/g, "");
+    str = str.replace(/\r\n?/g, "\n");
+    str = str.replace(/\n{3,}/g, "\n\n");
    str = str.trim();
    return str;
  }
@@ -7633,69 +7623,6 @@ var EnhancedSecureWebRTCManager = class _EnhancedSecureWebRTCManager {
      rateLimitBurstSize: 10
      // Burst size for rate limiting
    };
-    this._maliciousPatterns = [
-      // Enhanced script tag detection that handles edge cases
-      /<script\b[^>]*>[\s\S]*?<\/script\s*>/gi,
-      // Standard <\/script>
-      /<script\b[^>]*>[\s\S]*?<\/script\s+[^>]*>/gi,
-      // <\/script with attributes>
-      /<script\b[^>]*>[\s\S]*$/gi,
-      // Malformed script tags without closing
-      // Additional dangerous tags
-      /<iframe\b[^>]*>[\s\S]*?<\/iframe\s*>/gi,
-      // iframe tags
-      /<object\b[^>]*>[\s\S]*?<\/object\s*>/gi,
-      // object tags
-      /<embed\b[^>]*>/gi,
-      // embed tags
-      /<applet\b[^>]*>[\s\S]*?<\/applet\s*>/gi,
-      // applet tags
-      /<style\b[^>]*>[\s\S]*?<\/style\s*>/gi,
-      // style tags
-      // Dangerous protocols
-      /javascript\s*:/gi,
-      // JavaScript protocol
-      /data\s*:/gi,
-      // Data protocol
-      /vbscript\s*:/gi,
-      // VBScript protocol
-      /data:text\/html/gi,
-      // Data URLs with HTML
-      /on\w+\s*=/gi,
-      // Event handlers
-      /eval\s*\(/gi,
-      // eval() calls
-      /document\./gi,
-      // Document object access
-      /window\./gi,
-      // Window object access
-      /localStorage/gi,
-      // LocalStorage access
-      /sessionStorage/gi,
-      // SessionStorage access
-      /fetch\s*\(/gi,
-      // Fetch API calls
-      /XMLHttpRequest/gi,
-      // XHR calls
-      /import\s*\(/gi,
-      // Dynamic imports
-      /require\s*\(/gi,
-      // Require calls
-      /process\./gi,
-      // Process object access
-      /global/gi,
-      // Global object access
-      /__proto__/gi,
-      // Prototype pollution
-      /constructor/gi,
-      // Constructor access
-      /prototype/gi,
-      // Prototype access
-      /toString\s*\(/gi,
-      // toString calls
-      /valueOf\s*\(/gi
-      // valueOf calls
-    ];
    this._absoluteBlacklist = /* @__PURE__ */ new Set([
      // Cryptographic keys
      "encryptionKey",
@@ -9053,7 +8980,10 @@ var EnhancedSecureWebRTCManager = class _EnhancedSecureWebRTCManager {
      }
      return aad;
    } catch (error) {
-      this._secureLog("error", "AAD validation failed", { error: error.message, aadString });
+      this._secureLog("error", "AAD validation failed", {
+        error: error.message,
+        aadLength: typeof aadString === "string" ? aadString.length : 0
+      });
      throw new Error(`AAD validation failed: ${error.message}`);
    }
  }
@@ -16229,7 +16159,10 @@ var SecureKeyStorage = class {
      }
      return aad;
    } catch (error) {
-      this._secureLog("error", "AAD validation failed", { error: error.message, aadString });
+      this._secureLog("error", "AAD validation failed", {
+        error: error.message,
+        aadLength: typeof aadString === "string" ? aadString.length : 0
+      });
      throw new Error(`AAD validation failed: ${error.message}`);
    }
  }
@@ -11,7 +11,7 @@
    "dev": "npm run build && python -m http.server 8000",
    "watch": "npx tailwindcss -i src/styles/tw-input.css -o assets/tailwind.css --watch",
    "serve": "npx http-server -p 8000",
-    "test": "node tests/sas-verification.test.mjs && node tests/file-transfer-consent.test.mjs && node tests/incoming-message-sanitization.test.mjs && node tests/file-type-allowlist.test.mjs && node tests/webrtc-privacy-mode.test.mjs && node tests/indexeddb-metadata-encryption.test.mjs && node tests/disconnect-cleanup.test.mjs && node tests/timer-lifecycle.test.mjs && node tests/file-transfer-cleanup.test.mjs && node tests/file-transfer-ui-cleanup.test.mjs && node tests/file-transfer-callback-propagation.test.mjs && node tests/debug-window-hooks.test.mjs && node tests/inbound-message-rate-limit.test.mjs && node tests/file-transfer-chunk-rate-limit.test.mjs && node tests/ice-servers-validation.test.mjs"
+    "test": "node tests/sas-verification.test.mjs && node tests/file-transfer-consent.test.mjs && node tests/incoming-message-sanitization.test.mjs && node tests/outgoing-message-integrity.test.mjs && node tests/file-type-allowlist.test.mjs && node tests/webrtc-privacy-mode.test.mjs && node tests/indexeddb-metadata-encryption.test.mjs && node tests/disconnect-cleanup.test.mjs && node tests/timer-lifecycle.test.mjs && node tests/file-transfer-cleanup.test.mjs && node tests/file-transfer-ui-cleanup.test.mjs && node tests/file-transfer-callback-propagation.test.mjs && node tests/debug-window-hooks.test.mjs && node tests/inbound-message-rate-limit.test.mjs && node tests/file-transfer-chunk-rate-limit.test.mjs && node tests/ice-servers-validation.test.mjs"
  },
  "keywords": [
    "p2p",
@@ -1518,20 +1518,17 @@ this._secureLog('info', '🔒 Enhanced Mutex system fully initialized and valida
                    return validationResult;
                }

-                // 3. Malicious pattern detection for strings
-                for (const pattern of this._maliciousPatterns) {
-                    if (pattern.test(data)) {
-                        validationResult.errors.push(`Malicious pattern detected: ${pattern.source}`);
-                        this._secureLog('warn', '🚨 Malicious pattern detected in input', {
-                            context: context,
-                            pattern: pattern.source,
-                            dataLength: data.length
-                        });
-                        return validationResult;
-                    }
-                }
+                // 3. No keyword/markup blocklist here.
+                // This validates OUTGOING free-text chat content. A word-level
+                // blocklist (e.g. "constructor", "global", "document.", "javascript:")
+                // silently rejected legitimate messages while providing no real
+                // protection: the rendering boundary is the receive-side DOMPurify
+                // pass in deliverMessageToUI(), and outgoing text is additionally run
+                // through EnhancedSecureCryptoUtils.sanitizeMessage() before encryption.
+                // Markup a user types is neutralised there, not by guessing keywords.

                // 4. Sanitize string data
+
                validationResult.sanitizedData = this._sanitizeInputString(data);
                validationResult.isValid = true;
                return validationResult;
@@ -1639,16 +1636,24 @@ this._secureLog('info', '🔒 Enhanced Mutex system fully initialized and valida
     */
    _sanitizeInputString(str) {
        if (typeof str !== 'string') return str;
-        
-        // Remove null bytes
-        str = str.replace(/\0/g, '');
-        
-        // Normalize whitespace
-        str = str.replace(/\s+/g, ' ');
-        
-        // Trim
+
+        // Remove null bytes and C0/C1 control characters, but preserve the
+        // whitespace users actually type: newline (\n), carriage return (\r)
+        // and tab (\t). Collapsing all \s+ here destroyed multi-line messages
+        // and code snippets ("a\nb\nc" -> "a b c"), corrupting chat content.
+        str = str.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/g, '');
+
+        // Normalise line endings only. Internal spaces/tabs are preserved so
+        // code indentation, pasted keys and aligned text survive intact; the
+        // maxStringLength limit (and the receive-side 2000-char cap) bound abuse.
+        str = str.replace(/\r\n?/g, '\n');
+
+        // Collapse 3+ consecutive blank lines down to two.
+        str = str.replace(/\n{3,}/g, '\n\n');
+
+        // Trim leading/trailing whitespace only.
        str = str.trim();
-        
+
        return str;
    }

@@ -2089,41 +2094,12 @@ this._secureLog('info', '🔒 Enhanced Mutex system fully initialized and valida
            rateLimitBurstSize: 10 // Burst size for rate limiting
        };
        
-        //   Malicious pattern detection
-        this._maliciousPatterns = [
-            // Enhanced script tag detection that handles edge cases
-            /<script\b[^>]*>[\s\S]*?<\/script\s*>/gi, // Standard </script>
-            /<script\b[^>]*>[\s\S]*?<\/script\s+[^>]*>/gi, // </script with attributes>
-            /<script\b[^>]*>[\s\S]*$/gi, // Malformed script tags without closing
-            // Additional dangerous tags
-            /<iframe\b[^>]*>[\s\S]*?<\/iframe\s*>/gi, // iframe tags
-            /<object\b[^>]*>[\s\S]*?<\/object\s*>/gi, // object tags
-            /<embed\b[^>]*>/gi, // embed tags
-            /<applet\b[^>]*>[\s\S]*?<\/applet\s*>/gi, // applet tags
-            /<style\b[^>]*>[\s\S]*?<\/style\s*>/gi, // style tags
-            // Dangerous protocols
-            /javascript\s*:/gi, // JavaScript protocol
-            /data\s*:/gi, // Data protocol
-            /vbscript\s*:/gi, // VBScript protocol
-            /data:text\/html/gi, // Data URLs with HTML
-            /on\w+\s*=/gi, // Event handlers
-            /eval\s*\(/gi, // eval() calls
-            /document\./gi, // Document object access
-            /window\./gi, // Window object access
-            /localStorage/gi, // LocalStorage access
-            /sessionStorage/gi, // SessionStorage access
-            /fetch\s*\(/gi, // Fetch API calls
-            /XMLHttpRequest/gi, // XHR calls
-            /import\s*\(/gi, // Dynamic imports
-            /require\s*\(/gi, // Require calls
-            /process\./gi, // Process object access
-            /global/gi, // Global object access
-            /__proto__/gi, // Prototype pollution
-            /constructor/gi, // Constructor access
-            /prototype/gi, // Prototype access
-            /toString\s*\(/gi, // toString calls
-            /valueOf\s*\(/gi // valueOf calls
-        ];
+        // NOTE: A word/markup blocklist for outgoing chat text was removed.
+        // It rejected legitimate messages (plain words like "constructor",
+        // "global", "document.", or the literal text "javascript:") without
+        // adding security. XSS is prevented at the rendering boundary by the
+        // receive-side DOMPurify pass (deliverMessageToUI) and by sanitizeMessage()
+        // on the send path before encryption — not by keyword guessing.

        //   Comprehensive blacklist with all sensitive patterns
        this._absoluteBlacklist = new Set([
@@ -3796,7 +3772,12 @@ this._secureLog('info', '🔒 Enhanced Mutex system fully initialized and valida
            
            return aad;
        } catch (error) {
-            this._secureLog('error', 'AAD validation failed', { error: error.message, aadString });
+            // Never log the raw AAD: it carries sessionId and keyFingerprint.
+            // Length is enough to diagnose malformed input without leaking secrets.
+            this._secureLog('error', 'AAD validation failed', {
+                error: error.message,
+                aadLength: typeof aadString === 'string' ? aadString.length : 0
+            });
            throw new Error(`AAD validation failed: ${error.message}`);
        }
    }
@@ -13181,7 +13162,12 @@ class SecureKeyStorage {
            
            return aad;
        } catch (error) {
-            this._secureLog('error', 'AAD validation failed', { error: error.message, aadString });
+            // Never log the raw AAD: it carries sessionId and keyFingerprint.
+            // Length is enough to diagnose malformed input without leaking secrets.
+            this._secureLog('error', 'AAD validation failed', {
+                error: error.message,
+                aadLength: typeof aadString === 'string' ? aadString.length : 0
+            });
            throw new Error(`AAD validation failed: ${error.message}`);
        }
    }
@@ -0,0 +1,87 @@
+import assert from 'node:assert/strict';
+import { JSDOM } from 'jsdom';
+
+const { window } = new JSDOM('<!doctype html><html><body></body></html>', {
+    url: 'http://localhost/'
+});
+globalThis.window = window;
+
+const { EnhancedSecureCryptoUtils } = await import('../src/crypto/EnhancedSecureCryptoUtils.js');
+window.EnhancedSecureCryptoUtils = EnhancedSecureCryptoUtils;
+const { EnhancedSecureWebRTCManager } = await import('../src/network/EnhancedSecureWebRTCManager.js');
+
+const P = EnhancedSecureWebRTCManager.prototype;
+
+function ctx() {
+    return {
+        _inputValidationLimits: {
+            maxStringLength: 10000,
+            maxObjectDepth: 10,
+            maxArrayLength: 1000,
+            maxMessageSize: 1_000_000
+        },
+        _secureLog() {},
+        _sanitizeInputString: P._sanitizeInputString,
+        _sanitizeInputObject: P._sanitizeInputObject
+    };
+}
+
+function validate(input) {
+    return P._validateInputData.call(ctx(), input, 'sendSecureMessage');
+}
+
+// Legitimate plain-text messages that the old keyword blocklist rejected must
+// now be accepted unchanged. The real XSS boundary is the receive-side
+// DOMPurify pass, not a guess-the-keyword filter on outgoing content.
+for (const msg of [
+    'the constructor pattern is great',
+    'global warming is real',
+    'I will fetch (groceries) later',
+    'see document.pdf and check window.location',
+    'javascript: is harmless as plain text',
+    'discussing <script> tags and prototype chains',
+    'localStorage vs sessionStorage tradeoffs'
+]) {
+    const r = validate(msg);
+    assert.equal(r.isValid, true, `should accept: ${msg}`);
+    assert.equal(r.sanitizedData, msg, `should not mangle: ${msg}`);
+}
+
+// Multi-line content and indentation must survive (previously collapsed to one
+// line by replace(/\s+/g, ' ')).
+{
+    const multiline = 'line one\nline two\nline three';
+    const r = validate(multiline);
+    assert.equal(r.isValid, true);
+    assert.equal(r.sanitizedData, multiline, 'newlines must be preserved');
+}
+{
+    const code = 'function f() {\n    return 42;\n}';
+    const r = validate(code);
+    assert.equal(r.isValid, true);
+    assert.equal(r.sanitizedData, code, 'code indentation must be preserved');
+}
+
+// Control characters (null byte, bell, etc.) are still stripped, while tabs and
+// newlines are kept.
+{
+    const r = validate('a\u0000b\u0007c\td\ne');
+    assert.equal(r.isValid, true);
+    assert.equal(r.sanitizedData, 'abc\td\ne', 'control chars stripped, tab/newline kept');
+}
+
+// Excessive blank lines are collapsed but content stays intact.
+{
+    const r = validate('top\n\n\n\n\nbottom');
+    assert.equal(r.isValid, true);
+    assert.equal(r.sanitizedData, 'top\n\nbottom', '3+ blank lines collapse to two');
+}
+
+// Oversized input is still rejected (availability / DoS guard intact).
+{
+    const huge = 'a'.repeat(10001);
+    const r = validate(huge);
+    assert.equal(r.isValid, false, 'over-limit strings are rejected');
+}
+
+console.log('Outgoing message integrity tests passed');