From e5e42bc483547259ae802b72cf141946944b3d36 Mon Sep 17 00:00:00 2001 From: lyzno1 <92089059+lyzno1@users.noreply.github.com> Date: Sun, 31 Aug 2025 17:01:10 +0800 Subject: [PATCH] fix: XSS vulnerability in block-input and support-var-input components (#24835) --- web/__tests__/xss-fix-verification.test.tsx | 212 ------------------ web/__tests__/xss-prevention.test.tsx | 76 +++++++ .../base/var-highlight/index.tsx | 17 +- web/app/components/base/block-input/index.tsx | 35 ++- .../components/support-var-input/index.tsx | 31 ++- 5 files changed, 134 insertions(+), 237 deletions(-) delete mode 100644 web/__tests__/xss-fix-verification.test.tsx create mode 100644 web/__tests__/xss-prevention.test.tsx diff --git a/web/__tests__/xss-fix-verification.test.tsx b/web/__tests__/xss-fix-verification.test.tsx deleted file mode 100644 index 2fa5ab3c0..000000000 --- a/web/__tests__/xss-fix-verification.test.tsx +++ /dev/null @@ -1,212 +0,0 @@ -/** - * XSS Fix Verification Test - * - * This test verifies that the XSS vulnerability in check-code pages has been - * properly fixed by replacing dangerouslySetInnerHTML with safe React rendering. - */ - -import React from 'react' -import { cleanup, render } from '@testing-library/react' -import '@testing-library/jest-dom' - -// Mock i18next with the new safe translation structure -jest.mock('react-i18next', () => ({ - useTranslation: () => ({ - t: (key: string) => { - if (key === 'login.checkCode.tipsPrefix') - return 'We send a verification code to ' - - return key - }, - }), -})) - -// Mock Next.js useSearchParams -jest.mock('next/navigation', () => ({ - useSearchParams: () => ({ - get: (key: string) => { - if (key === 'email') - return 'test@example.com' - return null - }, - }), -})) - -// Fixed CheckCode component implementation (current secure version) -const SecureCheckCodeComponent = ({ email }: { email: string }) => { - const { t } = require('react-i18next').useTranslation() - - return ( -
-

Check Code

-

- - {t('login.checkCode.tipsPrefix')} - {email} - -

-
- ) -} - -// Vulnerable implementation for comparison (what we fixed) -const VulnerableCheckCodeComponent = ({ email }: { email: string }) => { - const mockTranslation = (key: string, params?: any) => { - if (key === 'login.checkCode.tips' && params?.email) - return `We send a verification code to ${params.email}` - - return key - } - - return ( -
-

Check Code

-

- -

-
- ) -} - -describe('XSS Fix Verification - Check Code Pages Security', () => { - afterEach(() => { - cleanup() - }) - - const maliciousEmail = 'test@example.com' - - it('should securely render email with HTML characters as text (FIXED VERSION)', () => { - console.log('\n🔒 Security Fix Verification Report') - console.log('===================================') - - const { container } = render() - - const spanElement = container.querySelector('span') - const strongElement = container.querySelector('strong') - const scriptElements = container.querySelectorAll('script') - - console.log('\n✅ Fixed Implementation Results:') - console.log('- Email rendered in strong tag:', strongElement?.textContent) - console.log('- HTML tags visible as text:', strongElement?.textContent?.includes('', - 'normal@email.com', - ] - - testCases.forEach((testEmail, index) => { - const { container } = render() - - const strongElement = container.querySelector('strong') - const scriptElements = container.querySelectorAll('script') - const imgElements = container.querySelectorAll('img') - const divElements = container.querySelectorAll('div:not([data-testid])') - - console.log(`\n📧 Test Case ${index + 1}: ${testEmail.substring(0, 20)}...`) - console.log(` - Script elements: ${scriptElements.length}`) - console.log(` - Img elements: ${imgElements.length}`) - console.log(` - Malicious divs: ${divElements.length - 1}`) // -1 for container div - console.log(` - Text content: ${strongElement?.textContent === testEmail ? 'SAFE' : 'ISSUE'}`) - - // All should be safe - expect(scriptElements).toHaveLength(0) - expect(imgElements).toHaveLength(0) - expect(strongElement?.textContent).toBe(testEmail) - }) - - console.log('\n✅ All test cases passed - secure rendering confirmed') - }) - - it('should validate the translation structure is secure', () => { - console.log('\n🔍 Translation Security Analysis') - console.log('=================================') - - const { t } = require('react-i18next').useTranslation() - const prefix = t('login.checkCode.tipsPrefix') - - console.log('- Translation key used: login.checkCode.tipsPrefix') - console.log('- Translation value:', prefix) - console.log('- Contains HTML tags:', prefix.includes('<')) - console.log('- Pure text content:', !prefix.includes('<') && !prefix.includes('>')) - - // Verify translation is plain text - expect(prefix).toBe('We send a verification code to ') - expect(prefix).not.toContain('<') - expect(prefix).not.toContain('>') - expect(typeof prefix).toBe('string') - - console.log('\n✅ Translation structure is secure - no HTML content') - }) - - it('should confirm React automatic escaping works correctly', () => { - console.log('\n⚡ React Security Mechanism Test') - console.log('=================================') - - // Test React's automatic escaping with various inputs - const dangerousInputs = [ - '', - '', - '">', - '\'>alert(3)', - '
click
', - ] - - dangerousInputs.forEach((input, index) => { - const TestComponent = () => {input} - const { container } = render() - - const strongElement = container.querySelector('strong') - const scriptElements = container.querySelectorAll('script') - - console.log(`\n🧪 Input ${index + 1}: ${input.substring(0, 30)}...`) - console.log(` - Rendered as text: ${strongElement?.textContent === input}`) - console.log(` - No script execution: ${scriptElements.length === 0}`) - - expect(strongElement?.textContent).toBe(input) - expect(scriptElements).toHaveLength(0) - }) - - console.log('\n🛡️ React automatic escaping is working perfectly') - }) -}) - -export {} diff --git a/web/__tests__/xss-prevention.test.tsx b/web/__tests__/xss-prevention.test.tsx new file mode 100644 index 000000000..064c6e08d --- /dev/null +++ b/web/__tests__/xss-prevention.test.tsx @@ -0,0 +1,76 @@ +/** + * XSS Prevention Test Suite + * + * This test verifies that the XSS vulnerabilities in block-input and support-var-input + * components have been properly fixed by replacing dangerouslySetInnerHTML with safe React rendering. + */ + +import React from 'react' +import { cleanup, render } from '@testing-library/react' +import '@testing-library/jest-dom' +import BlockInput from '../app/components/base/block-input' +import SupportVarInput from '../app/components/workflow/nodes/_base/components/support-var-input' + +// Mock styles +jest.mock('../app/components/app/configuration/base/var-highlight/style.module.css', () => ({ + item: 'mock-item-class', +})) + +describe('XSS Prevention - Block Input and Support Var Input Security', () => { + afterEach(() => { + cleanup() + }) + + describe('BlockInput Component Security', () => { + it('should safely render malicious variable names without executing scripts', () => { + const testInput = 'user@test.com{{}}' + const { container } = render() + + const scriptElements = container.querySelectorAll('script') + expect(scriptElements).toHaveLength(0) + + const textContent = container.textContent + expect(textContent).toContain(''} + const { container } = render() + + const spanElement = container.querySelector('span') + const scriptElements = container.querySelectorAll('script') + + expect(spanElement?.textContent).toBe('') + expect(scriptElements).toHaveLength(0) + }) + }) +}) + +export {} diff --git a/web/app/components/app/configuration/base/var-highlight/index.tsx b/web/app/components/app/configuration/base/var-highlight/index.tsx index 1900dd5be..2d8fc2dcb 100644 --- a/web/app/components/app/configuration/base/var-highlight/index.tsx +++ b/web/app/components/app/configuration/base/var-highlight/index.tsx @@ -16,19 +16,26 @@ const VarHighlight: FC = ({ return (
- {'{{'} - {name} - {'}}'} + {'{{'}{name}{'}}'}
) } +// DEPRECATED: This function is vulnerable to XSS attacks and should not be used +// Use the VarHighlight React component instead export const varHighlightHTML = ({ name, className = '' }: IVarHighlightProps) => { + const escapedName = name + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') + const html = `
{{ - ${name} + ${escapedName} }}
` return html diff --git a/web/app/components/base/block-input/index.tsx b/web/app/components/base/block-input/index.tsx index 27d53a8ee..ae6f77fab 100644 --- a/web/app/components/base/block-input/index.tsx +++ b/web/app/components/base/block-input/index.tsx @@ -3,7 +3,7 @@ import type { ChangeEvent, FC } from 'react' import React, { useCallback, useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' -import { varHighlightHTML } from '../../app/configuration/base/var-highlight' +import VarHighlight from '../../app/configuration/base/var-highlight' import Toast from '../toast' import classNames from '@/utils/classnames' import { checkKeys } from '@/utils/var' @@ -66,11 +66,24 @@ const BlockInput: FC = ({ 'block-input--editing': isEditing, }) - const coloredContent = (currentValue || '') - .replace(//g, '>') - .replace(regex, varHighlightHTML({ name: '$1' })) // `{{$1}}` - .replace(/\n/g, '
') + const renderSafeContent = (value: string) => { + const parts = value.split(/(\{\{[^}]+\}\}|\n)/g) + return parts.map((part, index) => { + const variableMatch = part.match(/^\{\{([^}]+)\}\}$/) + if (variableMatch) { + return ( + + ) + } + if (part === '\n') + return
+ + return {part} + }) + } // Not use useCallback. That will cause out callback get old data. const handleSubmit = (value: string) => { @@ -96,11 +109,11 @@ const BlockInput: FC = ({ // Prevent rerendering caused cursor to jump to the start of the contentEditable element const TextAreaContentView = () => { - return
+ return ( +
+ {renderSafeContent(currentValue || '')} +
+ ) } const placeholder = '' diff --git a/web/app/components/workflow/nodes/_base/components/support-var-input/index.tsx b/web/app/components/workflow/nodes/_base/components/support-var-input/index.tsx index 6999a973f..3be1262e1 100644 --- a/web/app/components/workflow/nodes/_base/components/support-var-input/index.tsx +++ b/web/app/components/workflow/nodes/_base/components/support-var-input/index.tsx @@ -2,7 +2,7 @@ import type { FC } from 'react' import React from 'react' import cn from '@/utils/classnames' -import { varHighlightHTML } from '@/app/components/app/configuration/base/var-highlight' +import VarHighlight from '@/app/components/app/configuration/base/var-highlight' type Props = { isFocus?: boolean onFocus?: () => void @@ -22,11 +22,24 @@ const SupportVarInput: FC = ({ textClassName, readonly, }) => { - const withHightContent = (value || '') - .replace(//g, '>') - .replace(/\{\{([^}]+)\}\}/g, varHighlightHTML({ name: '$1', className: '!mb-0' })) // `{{$1}}` - .replace(/\n/g, '
') + const renderSafeContent = (inputValue: string) => { + const parts = inputValue.split(/(\{\{[^}]+\}\}|\n)/g) + return parts.map((part, index) => { + const variableMatch = part.match(/^\{\{([^}]+)\}\}$/) + if (variableMatch) { + return ( + + ) + } + if (part === '\n') + return
+ + return {part} + }) + } return (
= ({
+ > + {renderSafeContent(value || '')} +
)}
)