test(audio): attacker-supplied input must not steer ffmpeg to other files or URLs (red)

A file's CONTENT can tell ffmpeg to open other paths or URLs (HLS, concat
playlists). ffmpeg 7.0.2 refuses the ones tried, but that is version luck and
the image's ffmpeg is still to be chosen (#34). Pin protocol_whitelist to
file,pipe and format_whitelist to the demuxers real audio uses, before the input.

Red: audio.AllowedDemuxers is undefined.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-01 17:13:54 +02:00
co-authored by Claude Sonnet 5.5
parent 1c726a810e
commit 4a0c51d877
2 changed files with 76 additions and 0 deletions
+19
View File
@@ -58,3 +58,22 @@ func TestFFmpegToWAV_MissingBinaryIsAnError(t *testing.T) {
require.Error(t, err) require.Error(t, err)
assert.False(t, errors.Is(err, context.Canceled)) assert.False(t, errors.Is(err, context.Canceled))
} }
func TestFFmpegToWAV_RestrictsWhatTheFileMayMakeFFmpegOpen(t *testing.T) {
rec := filepath.Join(t.TempDir(), "argv")
bin := stub(t, `printf '%s\n' "$@" > `+rec+`
for a; do last="$a"; done
printf data > "$last"`)
err := (&audio.FFmpeg{Bin: bin}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "o.wav"))
require.NoError(t, err)
args := argvOf(t, rec)
pw, fw, in := indexOf(args, "-protocol_whitelist"), indexOf(args, "-format_whitelist"), indexOf(args, "-i")
require.GreaterOrEqual(t, pw, 0, "argv: %v", args)
require.GreaterOrEqual(t, fw, 0, "argv: %v", args)
assert.Equal(t, "file,pipe", args[pw+1])
assert.Equal(t, audio.AllowedDemuxers, args[fw+1])
assert.Less(t, pw, in, "input options must come BEFORE -i or ffmpeg applies them to the output")
assert.Less(t, fw, in)
}
+57
View File
@@ -5,6 +5,7 @@ import (
"errors" "errors"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "time"
@@ -127,3 +128,59 @@ func TestFFprobeProbe_StopsWhenTheContextIsCancelled(t *testing.T) {
assert.Less(t, time.Since(start), 5*time.Second) assert.Less(t, time.Since(start), 5*time.Second)
assert.False(t, errors.Is(err, uploads.ErrUnreadable), "a timeout is not a verdict on the file") assert.False(t, errors.Is(err, uploads.ErrUnreadable), "a timeout is not a verdict on the file")
} }
// argvStub records the arguments it was called with into a file and then behaves as
// a successful ffprobe, so a test can assert exactly what the process was asked.
func argvStub(t *testing.T) (bin, argvFile string) {
t.Helper()
argvFile = filepath.Join(t.TempDir(), "argv")
bin = stub(t, `printf '%s\n' "$@" > `+argvFile+`
echo '{"streams":[{"codec_type":"audio"}],"format":{"duration":"1"}}'`)
return bin, argvFile
}
func argvOf(t *testing.T, file string) []string {
t.Helper()
raw, err := os.ReadFile(file)
require.NoError(t, err)
return strings.Split(strings.TrimSpace(string(raw)), "\n")
}
func indexOf(args []string, want string) int {
for i, a := range args {
if a == want {
return i
}
}
return -1
}
func TestFFprobeProbe_RestrictsWhatTheFileMayMakeFFmpegOpen(t *testing.T) {
// The input is attacker-supplied. ffmpeg can be told by a file's CONTENT to open
// other files or URLs (HLS and concat playlists); recent versions refuse by
// default, but that is version luck. Pin it: local files and pipes only, and
// only the demuxers real audio recordings use.
bin, argvFile := argvStub(t)
_, err := (&audio.FFprobe{Bin: bin}).Probe(context.Background(), "/spool/upload-1")
require.NoError(t, err)
args := argvOf(t, argvFile)
pw := indexOf(args, "-protocol_whitelist")
require.GreaterOrEqual(t, pw, 0, "argv: %v", args)
assert.Equal(t, "file,pipe", args[pw+1])
fw := indexOf(args, "-format_whitelist")
require.GreaterOrEqual(t, fw, 0, "argv: %v", args)
assert.Equal(t, audio.AllowedDemuxers, args[fw+1])
assert.Equal(t, "/spool/upload-1", args[len(args)-1], "the path stays the last argument")
}
func TestAllowedDemuxers_ExcludeThePlaylistFormats(t *testing.T) {
for _, bad := range []string{"concat", "hls", "applehttp", "dash", "rtsp", "sdp", "image2"} {
assert.NotContains(t, strings.Split(audio.AllowedDemuxers, ","), bad,
"%s can make ffmpeg open other files or URLs named inside the upload", bad)
}
for _, good := range []string{"wav", "mov", "mp4", "m4a", "mp3", "ogg", "matroska", "flac"} {
assert.Contains(t, strings.Split(audio.AllowedDemuxers, ","), good)
}
}