From 14297171bede133c62ba328fcf3f0878160d76b6 Mon Sep 17 00:00:00 2001
From: Rin <dangnhatrin90@gmail.com>
Date: Thu, 8 Jan 2026 12:33:50 +0700
Subject: [PATCH] Security: Enforce strict validation for FFmpeg binary paths
 (#214)

---
 backend/ffmpeg.go      | 57 ++++++++++++++++++++++++++++++++++++++----
 backend/filemanager.go |  4 +++
 backend/metadata.go    |  4 +++
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/backend/ffmpeg.go b/backend/ffmpeg.go
index 0597046..dc2d7ae 100644
--- a/backend/ffmpeg.go
+++ b/backend/ffmpeg.go
@@ -26,6 +26,49 @@ func decodeBase64(encoded string) (string, error) {
 	return string(decoded), nil
 }
 
+// ValidateExecutable checks if the path points to a valid, safe executable
+func ValidateExecutable(path string) error {
+	cleanedPath := filepath.Clean(path)
+	if cleanedPath == "" {
+		return fmt.Errorf("empty path")
+	}
+
+	// Ensure path is absolute
+	if !filepath.IsAbs(cleanedPath) {
+		return fmt.Errorf("path must be absolute: %s", path)
+	}
+
+	info, err := os.Stat(cleanedPath)
+	if err != nil {
+		return fmt.Errorf("failed to stat file: %w", err)
+	}
+
+	if info.IsDir() {
+		return fmt.Errorf("path is a directory: %s", path)
+	}
+
+	// Check executable permission on Unix
+	if runtime.GOOS != "windows" {
+		if info.Mode()&0111 == 0 {
+			return fmt.Errorf("file is not executable: %s", path)
+		}
+	}
+
+	// Validate filename allowlist
+	base := filepath.Base(cleanedPath)
+	validNames := map[string]bool{
+		"ffmpeg":      true,
+		"ffmpeg.exe":  true,
+		"ffprobe":     true,
+		"ffprobe.exe": true,
+	}
+	if !validNames[base] {
+		return fmt.Errorf("invalid executable name: %s", base)
+	}
+
+	return nil
+}
+
 const (
 	ffmpegWindowsURL = "aHR0cHM6Ly9naXRodWIuY29tL0J0Yk4vRkZtcGVnLUJ1aWxkcy9yZWxlYXNlcy9kb3dubG9hZC9sYXRlc3QvZmZtcGVnLW1hc3Rlci1sYXRlc3Qtd2luNjQtZ3BsLnppcA=="
 	ffmpegLinuxURL   = "aHR0cHM6Ly9naXRodWIuY29tL0J0Yk4vRkZtcGVnLUJ1aWxkcy9yZWxlYXNlcy9kb3dubG9hZC9sYXRlc3QvZmZtcGVnLW1hc3Rlci1sYXRlc3QtbGludXg2NC1ncGwudGFyLnh6"
@@ -84,6 +127,10 @@ func IsFFprobeInstalled() (bool, error) {
 		return false, nil
 	}
 
+	if err := ValidateExecutable(ffprobePath); err != nil {
+		return false, nil
+	}
+
 	// Verify it's executable
 	cmd := exec.Command(ffprobePath, "-version")
 	setHideWindow(cmd)
@@ -98,13 +145,9 @@ func IsFFmpegInstalled() (bool, error) {
 		return false, err
 	}
 
-	_, err = os.Stat(ffmpegPath)
-	if os.IsNotExist(err) {
+	if err := ValidateExecutable(ffmpegPath); err != nil {
 		return false, nil
 	}
-	if err != nil {
-		return false, err
-	}
 
 	// Verify it's executable
 	cmd := exec.Command(ffmpegPath, "-version")
@@ -425,6 +468,10 @@ func ConvertAudio(req ConvertAudioRequest) ([]ConvertAudioResult, error) {
 		return nil, fmt.Errorf("failed to get ffmpeg path: %w", err)
 	}
 
+	if err := ValidateExecutable(ffmpegPath); err != nil {
+		return nil, fmt.Errorf("invalid ffmpeg executable: %w", err)
+	}
+
 	installed, err := IsFFmpegInstalled()
 	if err != nil || !installed {
 		return nil, fmt.Errorf("ffmpeg is not installed")
diff --git a/backend/filemanager.go b/backend/filemanager.go
index 43e56df..7756473 100644
--- a/backend/filemanager.go
+++ b/backend/filemanager.go
@@ -244,6 +244,10 @@ func readMetadataWithFFprobe(filePath string) (*AudioMetadata, error) {
 		return nil, err
 	}
 
+	if err := ValidateExecutable(ffprobePath); err != nil {
+		return nil, fmt.Errorf("invalid ffprobe executable: %w", err)
+	}
+
 	// Use ffprobe to get metadata in JSON format (both format and stream tags)
 	cmd := exec.Command(ffprobePath,
 		"-v", "quiet",
diff --git a/backend/metadata.go b/backend/metadata.go
index 48f19c3..d057f48 100644
--- a/backend/metadata.go
+++ b/backend/metadata.go
@@ -541,6 +541,10 @@ func embedLyricsToM4A(filepath string, lyrics string) error {
 		return fmt.Errorf("ffmpeg not found: %w", err)
 	}
 
+	if err := ValidateExecutable(ffmpegPath); err != nil {
+		return fmt.Errorf("invalid ffmpeg executable: %w", err)
+	}
+
 	// Create temporary output file with proper extension so ffmpeg can detect format
 	tmpOutputFile := strings.TrimSuffix(filepath, pathfilepath.Ext(filepath)) + ".tmp" + pathfilepath.Ext(filepath)
 	defer func() {