浏览代码

security: Implement FFmpeg injection prevention & URL validation (M3 & M4)

Implemented two MEDIUM priority security fixes:

M3: FFmpeg Command Injection Prevention
- Created validateFFmpegFormat() - whitelists H264, ProRes, DNxHR, Audio only
- Created validateFFmpegQuality() - whitelists 4K, 1440p, 1080p, 720p, 480p, 360p
- Created validateFFmpegExtension() - safe file extension mapping
- Applied validation in src/main.js convertVideoFormat()
- Prevents command injection via format/quality parameters

M4: URL Validation Enhancement
- Added isPunycode() - detects "xn--" punycode domains
- Added detectHomographAttack() - detects Cyrillic/Greek lookalike chars
- Added validateUrlSecurity() - comprehensive security validation
- Integrated into isValidVideoUrl() with trusted domain check
- Blocks suspicious URLs while allowing trusted domains

Security Improvements:
✅ FFmpeg format/quality whitelisting prevents arbitrary commands
✅ Punycode domain detection (xn--youtube.com → BLOCKED)
✅ Homograph attack detection (youtubе.com with Cyrillic е → BLOCKED)
✅ Mixed script detection (ASCII + non-ASCII → WARNING)
✅ Trusted domain validation (youtube.com, youtu.be, vimeo.com)

All 259 tests passing. No regressions.

Issues: #11 (Security: M3 - FFmpeg Injection, M4 - URL Validation)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
jopa79 3 月之前
父节点
当前提交
94dada16e1
共有 3 个文件被更改,包括 241 次插入7 次删除
  1. 137 0
      scripts/utils/url-validator.js
  2. 15 6
      src/main.js
  3. 89 1
      src/security-utils.js

+ 137 - 0
scripts/utils/url-validator.js

@@ -2,6 +2,133 @@
 // Comprehensive URL validation for video platforms
 
 class URLValidator {
+    /**
+     * Detect punycode/IDN domains that might be used for spoofing
+     * @param {string} hostname - Domain hostname to check
+     * @returns {boolean} True if punycode detected
+     */
+    static isPunycode(hostname) {
+        if (!hostname || typeof hostname !== 'string') {
+            return false;
+        }
+
+        // Punycode domains start with "xn--"
+        return hostname.toLowerCase().includes('xn--');
+    }
+
+    /**
+     * Detect potential homograph attacks using lookalike characters
+     * @param {string} hostname - Domain hostname to check
+     * @returns {object} Detection result with suspicious characters
+     */
+    static detectHomographAttack(hostname) {
+        if (!hostname || typeof hostname !== 'string') {
+            return { suspicious: false, characters: [] };
+        }
+
+        // Common homograph character mappings
+        const suspiciousChars = [
+            // Cyrillic lookalikes
+            { char: 'а', lookalike: 'a', name: 'Cyrillic а' },
+            { char: 'е', lookalike: 'e', name: 'Cyrillic е' },
+            { char: 'о', lookalike: 'o', name: 'Cyrillic о' },
+            { char: 'р', lookalike: 'p', name: 'Cyrillic р' },
+            { char: 'с', lookalike: 'c', name: 'Cyrillic с' },
+            { char: 'у', lookalike: 'y', name: 'Cyrillic у' },
+            { char: 'х', lookalike: 'x', name: 'Cyrillic х' },
+
+            // Greek lookalikes
+            { char: 'ο', lookalike: 'o', name: 'Greek omicron' },
+            { char: 'ν', lookalike: 'v', name: 'Greek nu' },
+            { char: 'α', lookalike: 'a', name: 'Greek alpha' },
+
+            // Other suspicious characters
+            { char: 'і', lookalike: 'i', name: 'Ukrainian i' },
+            { char: 'ј', lookalike: 'j', name: 'Cyrillic j' }
+        ];
+
+        const found = [];
+        const lowerHostname = hostname.toLowerCase();
+
+        for (const { char, lookalike, name } of suspiciousChars) {
+            if (lowerHostname.includes(char)) {
+                found.push({ char, lookalike, name });
+            }
+        }
+
+        // Check for mixed scripts (ASCII + non-ASCII)
+        const hasAscii = /[a-z]/i.test(hostname);
+        const hasNonAscii = /[^\x00-\x7F]/.test(hostname);
+        const mixedScripts = hasAscii && hasNonAscii;
+
+        return {
+            suspicious: found.length > 0 || mixedScripts,
+            characters: found,
+            mixedScripts
+        };
+    }
+
+    /**
+     * Validate URL against security threats (punycode, homographs)
+     * @param {string} url - URL to validate
+     * @returns {object} Validation result with security warnings
+     */
+    static validateUrlSecurity(url) {
+        if (!url || typeof url !== 'string') {
+            return { safe: false, warnings: ['Invalid URL'] };
+        }
+
+        try {
+            const parsed = new URL(url.startsWith('http') ? url : 'https://' + url);
+            const hostname = parsed.hostname.toLowerCase();
+            const warnings = [];
+
+            // Check for punycode
+            if (this.isPunycode(hostname)) {
+                warnings.push('⚠️ Punycode domain detected - may be used for spoofing');
+            }
+
+            // Check for homograph attacks
+            const homograph = this.detectHomographAttack(hostname);
+            if (homograph.suspicious) {
+                if (homograph.characters.length > 0) {
+                    warnings.push(`⚠️ Suspicious lookalike characters detected: ${homograph.characters.map(c => c.name).join(', ')}`);
+                }
+                if (homograph.mixedScripts) {
+                    warnings.push('⚠️ Mixed character scripts detected - potential homograph attack');
+                }
+            }
+
+            // Verify against trusted domains
+            const trustedDomains = [
+                'youtube.com',
+                'youtu.be',
+                'vimeo.com'
+            ];
+
+            const isTrusted = trustedDomains.some(domain =>
+                hostname === domain ||
+                hostname.endsWith('.' + domain) ||
+                hostname === 'www.' + domain
+            );
+
+            if (!isTrusted && warnings.length === 0) {
+                warnings.push('⚠️ Domain not in trusted list');
+            }
+
+            return {
+                safe: warnings.length === 0,
+                warnings,
+                hostname,
+                isTrusted
+            };
+        } catch (error) {
+            return {
+                safe: false,
+                warnings: ['Invalid URL format']
+            };
+        }
+    }
     // Check if URL is a valid video URL from supported platforms
     static isValidVideoUrl(url) {
         if (!url || typeof url !== 'string') {
@@ -13,6 +140,16 @@ class URLValidator {
             return false;
         }
 
+        // SECURITY: Check for punycode and homograph attacks
+        const securityCheck = this.validateUrlSecurity(trimmedUrl);
+        if (!securityCheck.safe) {
+            // Log security warnings but still allow if domain is trusted
+            if (!securityCheck.isTrusted) {
+                console.warn('URL security warnings:', securityCheck.warnings);
+                return false;
+            }
+        }
+
         // Check against supported platforms
         return this.isYouTubeUrl(trimmedUrl) ||
                this.isVimeoUrl(trimmedUrl) ||

+ 15 - 6
src/main.js

@@ -5,7 +5,7 @@ const { spawn } = require('child_process')
 const notifier = require('node-notifier')
 const ffmpegConverter = require('../scripts/utils/ffmpeg-converter')
 const DownloadManager = require('./download-manager')
-const { sanitizePath, validateCookieFile, sanitizeFilename, isValidVideoUrl } = require('./security-utils')
+const { sanitizePath, validateCookieFile, sanitizeFilename, isValidVideoUrl, validateFFmpegFormat, validateFFmpegQuality, validateFFmpegExtension } = require('./security-utils')
 const logger = require('./logger')
 
 // Keep a global reference of the window object
@@ -1433,9 +1433,18 @@ async function convertVideoFormat(event, { url, inputPath, format, quality, save
     throw new Error('FFmpeg binary not found - conversion not available')
   }
 
+  // SECURITY: Validate format and quality parameters to prevent command injection
+  let validatedFormat, validatedQuality
+  try {
+    validatedFormat = validateFFmpegFormat(format)
+    validatedQuality = validateFFmpegQuality(quality)
+  } catch (error) {
+    throw new Error(`Invalid conversion parameters: ${error.message}`)
+  }
+
   // Generate output filename with appropriate extension and format suffix
   const inputFilename = path.basename(inputPath, path.extname(inputPath))
-  const outputExtension = getOutputExtension(format)
+  const outputExtension = validateFFmpegExtension(validatedFormat)
 
   // Map format names to proper filename suffixes
   const formatSuffixes = {
@@ -1444,13 +1453,13 @@ async function convertVideoFormat(event, { url, inputPath, format, quality, save
     'DNxHR': 'dnxhd',
     'Audio only': 'audio'
   }
-  const suffix = formatSuffixes[format] || format.toLowerCase()
+  const suffix = formatSuffixes[validatedFormat] || validatedFormat.toLowerCase()
 
   const outputFilename = `${inputFilename}_${suffix}.${outputExtension}`
   const outputPath = path.join(savePath, outputFilename)
 
   logger.debug('Starting format conversion:', {
-    inputPath, outputPath, format, quality
+    inputPath, outputPath, format: validatedFormat, quality: validatedQuality
   })
 
   // Get video duration for progress calculation
@@ -1482,8 +1491,8 @@ async function convertVideoFormat(event, { url, inputPath, format, quality, save
     const result = await ffmpegConverter.convertVideo({
       inputPath,
       outputPath,
-      format,
-      quality,
+      format: validatedFormat,
+      quality: validatedQuality,
       duration,
       onProgress
     })

+ 89 - 1
src/security-utils.js

@@ -183,9 +183,97 @@ function isValidVideoUrl(url) {
   }
 }
 
+/**
+ * Validate and sanitize FFmpeg format parameter
+ * Prevents command injection via format parameter
+ * @param {string} format - Format string from user
+ * @returns {string} Validated format
+ * @throws {Error} If format is invalid
+ */
+function validateFFmpegFormat(format) {
+  if (!format || typeof format !== 'string') {
+    throw new Error('Invalid format parameter');
+  }
+
+  // Whitelist of allowed formats
+  const allowedFormats = [
+    'H264',
+    'ProRes',
+    'DNxHR',
+    'Audio only',
+    'None'
+  ];
+
+  const trimmed = format.trim();
+
+  if (!allowedFormats.includes(trimmed)) {
+    throw new Error(`Invalid format: ${trimmed}. Allowed formats: ${allowedFormats.join(', ')}`);
+  }
+
+  return trimmed;
+}
+
+/**
+ * Validate and sanitize FFmpeg quality parameter
+ * Prevents command injection via quality parameter
+ * @param {string} quality - Quality string from user
+ * @returns {string} Validated quality
+ * @throws {Error} If quality is invalid
+ */
+function validateFFmpegQuality(quality) {
+  if (!quality || typeof quality !== 'string') {
+    throw new Error('Invalid quality parameter');
+  }
+
+  // Whitelist of allowed quality settings
+  const allowedQualities = [
+    '4K',
+    '1440p',
+    '1080p',
+    '720p',
+    '480p',
+    '360p',
+    'best',
+    'worst'
+  ];
+
+  const trimmed = quality.trim();
+
+  if (!allowedQualities.includes(trimmed)) {
+    throw new Error(`Invalid quality: ${trimmed}. Allowed qualities: ${allowedQualities.join(', ')}`);
+  }
+
+  return trimmed;
+}
+
+/**
+ * Validate FFmpeg output file extension
+ * @param {string} format - Target format
+ * @returns {string} Safe file extension
+ */
+function validateFFmpegExtension(format) {
+  const extensionMap = {
+    'H264': 'mp4',
+    'ProRes': 'mov',
+    'DNxHR': 'mov',
+    'Audio only': 'm4a',
+    'None': '' // No conversion
+  };
+
+  const ext = extensionMap[format];
+  if (!ext && format !== 'None') {
+    throw new Error(`Unknown format for extension mapping: ${format}`);
+  }
+
+  return ext;
+}
+
 module.exports = {
   sanitizePath,
   validateCookieFile,
   sanitizeFilename,
-  isValidVideoUrl
+  isValidVideoUrl,
+  validateFFmpegFormat,
+  validateFFmpegQuality,
+  validateFFmpegExtension
 };