diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt index 6da15cce..5b4b607e 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt @@ -87,12 +87,16 @@ internal fun NativeDownloadFinalizer.scanReplayGain(path: String, shouldCancel: } internal fun NativeDownloadFinalizer.runFFmpeg(command: String, shouldCancel: () -> Boolean = { false }): Pair { + return runFFmpegArguments(FFmpegKitConfig.parseArguments(command), shouldCancel) +} + +internal fun NativeDownloadFinalizer.runFFmpegArguments(arguments: Array, shouldCancel: () -> Boolean = { false }): Pair { checkCancelled(shouldCancel) installNativeFFmpegCallbackFilter() val latch = CountDownLatch(1) var completedSession: FFmpegSession? = null val session = FFmpegSession.create( - FFmpegKitConfig.parseArguments(command), + arguments, { finishedSession -> completedSession = finishedSession latch.countDown() @@ -158,8 +162,15 @@ internal fun NativeDownloadFinalizer.withFFmpegCommandPump( for (index in 0 until commands.length()) { val command = commands.optJSONObject(index) ?: continue val id = command.optString("command_id", "") - val commandLine = command.optString("command", "") - if (id.isBlank() || commandLine.isBlank() || handled.contains(id)) { + val rawArguments = command.optJSONArray("arguments") + val arguments = if (rawArguments == null) { + emptyArray() + } else { + Array(rawArguments.length()) { argumentIndex -> + rawArguments.optString(argumentIndex, "") + } + } + if (id.isBlank() || arguments.isEmpty() || arguments.any { it.isEmpty() } || handled.contains(id)) { continue } handled.add(id) @@ -172,7 +183,7 @@ internal fun NativeDownloadFinalizer.withFFmpegCommandPump( if (shouldCancel()) { Pair(false, "cancelled") } else { - runFFmpeg(commandLine, shouldCancel) + runFFmpegArguments(arguments, shouldCancel) } } catch (e: Exception) { Pair(false, e.message ?: "FFmpeg execution failed") diff --git a/go_backend/exports_extensions.go b/go_backend/exports_extensions.go index 2001dd65..b5831580 100644 --- a/go_backend/exports_extensions.go +++ b/go_backend/exports_extensions.go @@ -702,7 +702,7 @@ func GetPendingFFmpegCommandJSON(commandID string) (string, error) { result := map[string]any{ "command_id": commandID, "extension_id": cmd.ExtensionID, - "command": cmd.Command, + "arguments": cmd.Arguments, "input_path": cmd.InputPath, "output_path": cmd.OutputPath, } @@ -724,7 +724,7 @@ func GetAllPendingFFmpegCommandsJSON() (string, error) { commands = append(commands, map[string]any{ "command_id": cmdID, "extension_id": cmd.ExtensionID, - "command": cmd.Command, + "arguments": cmd.Arguments, }) } } @@ -754,7 +754,7 @@ func WaitForPendingFFmpegCommandsJSON(timeoutMillis int64) (string, error) { commands = append(commands, map[string]any{ "command_id": cmdID, "extension_id": cmd.ExtensionID, - "command": cmd.Command, + "arguments": cmd.Arguments, }) } ffmpegCommandsMu.Unlock() diff --git a/go_backend/exports_supplement_test.go b/go_backend/exports_supplement_test.go index f39f1099..cc73276e 100644 --- a/go_backend/exports_supplement_test.go +++ b/go_backend/exports_supplement_test.go @@ -403,7 +403,7 @@ func TestExportsJSONWrappersAndExtensionManagerSurface(t *testing.T) { } ffmpegCommandsMu.Lock() - ffmpegCommands["cmd-1"] = &FFmpegCommand{ExtensionID: ext.ID, Command: "ffmpeg -version", InputPath: "in", OutputPath: "out"} + ffmpegCommands["cmd-1"] = &FFmpegCommand{ExtensionID: ext.ID, Arguments: []string{"-version"}, InputPath: "in", OutputPath: "out"} ffmpegCommandsMu.Unlock() if cmdJSON, err := GetPendingFFmpegCommandJSON("cmd-1"); err != nil || !strings.Contains(cmdJSON, "cmd-1") { t.Fatalf("GetPendingFFmpegCommandJSON = %q/%v", cmdJSON, err) diff --git a/go_backend/extension_runtime_ffmpeg.go b/go_backend/extension_runtime_ffmpeg.go index c2f906c1..2f449735 100644 --- a/go_backend/extension_runtime_ffmpeg.go +++ b/go_backend/extension_runtime_ffmpeg.go @@ -2,7 +2,7 @@ package gobackend import ( "fmt" - "strings" + "regexp" "sync" "time" @@ -12,7 +12,7 @@ import ( // FFmpegCommand holds a pending FFmpeg command for Flutter to execute. type FFmpegCommand struct { ExtensionID string - Command string + Arguments []string InputPath string OutputPath string Completed bool @@ -67,24 +67,21 @@ func ClearFFmpegCommand(commandID string) { } func (r *extensionRuntime) ffmpegExecute(call goja.FunctionCall) goja.Value { - if r.manifest == nil || !r.manifest.Permissions.File || !r.manifest.HasCapability("rawFfmpeg") { - return r.jsError("raw FFmpeg execution permission denied") - } - if len(call.Arguments) < 1 { - return r.jsError("command is required") - } - - return r.executeFFmpegCommand(call.Arguments[0].String(), "", "") + // A raw command can introduce additional file inputs and network protocols, + // bypassing both validatePath and the extension network allow-list. Keep the + // API stub for compatibility, but never forward an unstructured command to + // the native FFmpeg process. + return r.jsError("raw FFmpeg execution is disabled; use ffmpeg.convert") } -func (r *extensionRuntime) executeFFmpegCommand(command, inputPath, outputPath string) goja.Value { +func (r *extensionRuntime) executeFFmpegCommand(arguments []string, inputPath, outputPath string) goja.Value { ffmpegCommandsMu.Lock() ffmpegCommandID++ cmdID := fmt.Sprintf("%s_%d", r.extensionID, ffmpegCommandID) queuedCommand := &FFmpegCommand{ ExtensionID: r.extensionID, - Command: command, + Arguments: append([]string(nil), arguments...), InputPath: inputPath, OutputPath: outputPath, Completed: false, @@ -115,6 +112,20 @@ func (r *extensionRuntime) executeFFmpegCommand(command, inputPath, outputPath s } } +var ffmpegBitratePattern = regexp.MustCompile(`^[1-9][0-9]{0,7}[kKmM]?$`) + +var allowedFFmpegAudioCodecs = map[string]struct{}{ + "aac": {}, + "alac": {}, + "copy": {}, + "flac": {}, + "libmp3lame": {}, + "libopus": {}, + "opus": {}, + "pcm_s16le": {}, + "pcm_s24le": {}, +} + func (r *extensionRuntime) ffmpegGetInfo(call goja.FunctionCall) goja.Value { if r.manifest == nil || !r.manifest.Permissions.File { return r.jsError("file permission denied") @@ -166,28 +177,37 @@ func (r *extensionRuntime) ffmpegConvert(call goja.FunctionCall) goja.Value { } } - var cmdParts []string - cmdParts = append(cmdParts, "-i", fmt.Sprintf("%q", inputPath)) + arguments := []string{"-hide_banner", "-nostdin", "-i", inputPath} if codec, ok := options["codec"].(string); ok { - cmdParts = append(cmdParts, "-c:a", codec) + if _, allowed := allowedFFmpegAudioCodecs[codec]; !allowed { + return r.jsError("unsupported audio codec") + } + arguments = append(arguments, "-c:a", codec) } if bitrate, ok := options["bitrate"].(string); ok { - cmdParts = append(cmdParts, "-b:a", bitrate) + if !ffmpegBitratePattern.MatchString(bitrate) { + return r.jsError("invalid audio bitrate") + } + arguments = append(arguments, "-b:a", bitrate) } if sampleRate, ok := options["sample_rate"].(float64); ok { - cmdParts = append(cmdParts, "-ar", fmt.Sprintf("%d", int(sampleRate))) + if sampleRate < 8_000 || sampleRate > 768_000 || sampleRate != float64(int(sampleRate)) { + return r.jsError("invalid sample rate") + } + arguments = append(arguments, "-ar", fmt.Sprintf("%d", int(sampleRate))) } if channels, ok := options["channels"].(float64); ok { - cmdParts = append(cmdParts, "-ac", fmt.Sprintf("%d", int(channels))) + if channels < 1 || channels > 32 || channels != float64(int(channels)) { + return r.jsError("invalid channel count") + } + arguments = append(arguments, "-ac", fmt.Sprintf("%d", int(channels))) } - cmdParts = append(cmdParts, "-y", fmt.Sprintf("%q", outputPath)) + arguments = append(arguments, "-y", outputPath) - command := strings.Join(cmdParts, " ") - - return r.executeFFmpegCommand(command, inputPath, outputPath) + return r.executeFFmpegCommand(arguments, inputPath, outputPath) } diff --git a/go_backend/extension_runtime_ffmpeg_test.go b/go_backend/extension_runtime_ffmpeg_test.go index 42ae6fca..bc7627fa 100644 --- a/go_backend/extension_runtime_ffmpeg_test.go +++ b/go_backend/extension_runtime_ffmpeg_test.go @@ -3,13 +3,15 @@ package gobackend import ( "strings" "testing" + + "github.com/dop251/goja" ) func TestWaitForPendingFFmpegCommandsClaimsCommandOnce(t *testing.T) { const commandID = "wait-claim-test" command := &FFmpegCommand{ ExtensionID: "test-extension", - Command: "ffmpeg -version", + Arguments: []string{"-version"}, done: make(chan struct{}), } ffmpegCommandsMu.Lock() @@ -34,3 +36,41 @@ func TestWaitForPendingFFmpegCommandsClaimsCommandOnce(t *testing.T) { t.Fatal("command completion did not signal waiter") } } + +func TestExtensionFFmpegRejectsRawAndInjectedOptions(t *testing.T) { + vm := goja.New() + runtime := &extensionRuntime{ + extensionID: "ffmpeg-security", + manifest: &ExtensionManifest{ + Permissions: ExtensionPermissions{File: true}, + Capabilities: map[string]any{"rawFfmpeg": true}, + }, + dataDir: t.TempDir(), + vm: vm, + } + raw := runtime.ffmpegExecute(goja.FunctionCall{Arguments: []goja.Value{ + vm.ToValue("-i /private/secret -f data out"), + }}).Export().(map[string]any) + if raw["success"] != false || !strings.Contains(raw["error"].(string), "disabled") { + t.Fatalf("raw FFmpeg was not rejected: %#v", raw) + } + + injected := runtime.ffmpegConvert(goja.FunctionCall{Arguments: []goja.Value{ + vm.ToValue("input.flac"), + vm.ToValue("output.flac"), + vm.ToValue(map[string]any{"codec": "flac -i /private/secret"}), + }}).Export().(map[string]any) + if injected["success"] != false || !strings.Contains(injected["error"].(string), "unsupported") { + t.Fatalf("FFmpeg option injection was not rejected: %#v", injected) + } + + injectedBitrate := runtime.ffmpegConvert(goja.FunctionCall{Arguments: []goja.Value{ + vm.ToValue("input.flac"), + vm.ToValue("output.m4a"), + vm.ToValue(map[string]any{"bitrate": "320k -i /private/secret"}), + }}).Export().(map[string]any) + if injectedBitrate["success"] != false || + !strings.Contains(injectedBitrate["error"].(string), "bitrate") { + t.Fatalf("FFmpeg bitrate injection was not rejected: %#v", injectedBitrate) + } +}