From cd93a5baf35f9879a3a354ed4d196235af692142 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 30 Aug 2024 14:03:59 +0200 Subject: [PATCH] [security] Implement `allowFiles` fs for better isolation of ffmpeg / ffprobe (#3251) * [chore] Implement readOneFile fs * further isolation * remove fmt call * tweaks --- internal/media/ffmpeg.go | 88 ++++++++++++++++++++++++++-------------- internal/media/util.go | 47 +++++++++++++++++++++ 2 files changed, 104 insertions(+), 31 deletions(-) diff --git a/internal/media/ffmpeg.go b/internal/media/ffmpeg.go index 0443f95b8..58c2f9503 100644 --- a/internal/media/ffmpeg.go +++ b/internal/media/ffmpeg.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "errors" + "os" "path" "strconv" "strings" @@ -39,10 +40,7 @@ import ( // any metadata encoded into the media stream itself will not be cleared. This is the best we // can do without absolutely tanking performance by requiring transcodes :( func ffmpegClearMetadata(ctx context.Context, outpath, inpath string) error { - // Get directory from filepath. - dirpath := path.Dir(inpath) - - return ffmpeg(ctx, dirpath, + return ffmpeg(ctx, inpath, outpath, // Only log errors. "-loglevel", "error", @@ -66,18 +64,15 @@ func ffmpegClearMetadata(ctx context.Context, outpath, inpath string) error { } // ffmpegGenerateWebpThumb generates a thumbnail webp from input media of any type, useful for any media. -func ffmpegGenerateWebpThumb(ctx context.Context, filepath, outpath string, width, height int, pixfmt string) error { - // Get directory from filepath. - dirpath := path.Dir(filepath) - +func ffmpegGenerateWebpThumb(ctx context.Context, inpath, outpath string, width, height int, pixfmt string) error { // Generate thumb with ffmpeg. - return ffmpeg(ctx, dirpath, + return ffmpeg(ctx, inpath, outpath, // Only log errors. "-loglevel", "error", // Input file. - "-i", filepath, + "-i", inpath, // Encode using libwebp. // (NOT as libwebp_anim). @@ -116,27 +111,24 @@ func ffmpegGenerateWebpThumb(ctx context.Context, filepath, outpath string, widt } // ffmpegGenerateStatic generates a static png from input image of any type, useful for emoji. -func ffmpegGenerateStatic(ctx context.Context, filepath string) (string, error) { +func ffmpegGenerateStatic(ctx context.Context, inpath string) (string, error) { var outpath string // Generate thumb output path REPLACING extension. - if i := strings.IndexByte(filepath, '.'); i != -1 { - outpath = filepath[:i] + "_static.png" + if i := strings.IndexByte(inpath, '.'); i != -1 { + outpath = inpath[:i] + "_static.png" } else { return "", gtserror.New("input file missing extension") } - // Get directory from filepath. - dirpath := path.Dir(filepath) - // Generate static with ffmpeg. - if err := ffmpeg(ctx, dirpath, + if err := ffmpeg(ctx, inpath, outpath, // Only log errors. "-loglevel", "error", // Input file. - "-i", filepath, + "-i", inpath, // Only first frame. "-frames:v", "1", @@ -157,18 +149,45 @@ func ffmpegGenerateStatic(ctx context.Context, filepath string) (string, error) return outpath, nil } -// ffmpeg calls `ffmpeg [args...]` (WASM) with directory path mounted in runtime. -func ffmpeg(ctx context.Context, dirpath string, args ...string) error { +// ffmpeg calls `ffmpeg [args...]` (WASM) with in + out paths mounted in runtime. +func ffmpeg(ctx context.Context, inpath string, outpath string, args ...string) error { var stderr byteutil.Buffer rc, err := _ffmpeg.Ffmpeg(ctx, _ffmpeg.Args{ Stderr: &stderr, Args: args, Config: func(modcfg wazero.ModuleConfig) wazero.ModuleConfig { - fscfg := wazero.NewFSConfig() // needs /dev/urandom - fscfg = fscfg.WithReadOnlyDirMount("/dev", "/dev") - fscfg = fscfg.WithDirMount(dirpath, dirpath) - modcfg = modcfg.WithFSConfig(fscfg) - return modcfg + fscfg := wazero.NewFSConfig() + + // Needs read-only access to + // /dev/urandom for some types. + urandom := &allowFiles{ + { + abs: "/dev/urandom", + flag: os.O_RDONLY, + perm: 0, + }, + } + fscfg = fscfg.WithFSMount(urandom, "/dev") + + // In+out dirs are always the same (tmp), + // so we can share one file system for + // both + grant different perms to inpath + // (read only) and outpath (read+write). + shared := &allowFiles{ + { + abs: inpath, + flag: os.O_RDONLY, + perm: 0, + }, + { + abs: outpath, + flag: os.O_RDWR | os.O_CREATE | os.O_TRUNC, + perm: 0666, + }, + } + fscfg = fscfg.WithFSMount(shared, path.Dir(inpath)) + + return modcfg.WithFSConfig(fscfg) }, }) if err != nil { @@ -183,9 +202,6 @@ func ffmpeg(ctx context.Context, dirpath string, args ...string) error { func ffprobe(ctx context.Context, filepath string) (*result, error) { var stdout byteutil.Buffer - // Get directory from filepath. - dirpath := path.Dir(filepath) - // Run ffprobe on our given file at path. _, err := _ffmpeg.Ffprobe(ctx, _ffmpeg.Args{ Stdout: &stdout, @@ -222,9 +238,19 @@ func ffprobe(ctx context.Context, filepath string) (*result, error) { Config: func(modcfg wazero.ModuleConfig) wazero.ModuleConfig { fscfg := wazero.NewFSConfig() - fscfg = fscfg.WithReadOnlyDirMount(dirpath, dirpath) - modcfg = modcfg.WithFSConfig(fscfg) - return modcfg + + // Needs read-only access + // to file being probed. + in := &allowFiles{ + { + abs: filepath, + flag: os.O_RDONLY, + perm: 0, + }, + } + fscfg = fscfg.WithFSMount(in, path.Dir(filepath)) + + return modcfg.WithFSConfig(fscfg) }, }) if err != nil { diff --git a/internal/media/util.go b/internal/media/util.go index f743e3821..22121a546 100644 --- a/internal/media/util.go +++ b/internal/media/util.go @@ -22,13 +22,60 @@ import ( "errors" "fmt" "io" + "io/fs" "os" + "path" "codeberg.org/gruf/go-bytesize" "codeberg.org/gruf/go-iotools" "codeberg.org/gruf/go-mimetypes" ) +// file represents one file +// with the given flag and perms. +type file struct { + abs string + flag int + perm os.FileMode +} + +// allowFiles implements fs.FS to allow +// access to a specified slice of files. +type allowFiles []file + +// Open implements fs.FS. +func (af allowFiles) Open(name string) (fs.File, error) { + for _, file := range af { + var ( + abs = file.abs + flag = file.flag + perm = file.perm + ) + + // Allowed to open file + // at absolute path. + if name == file.abs { + return os.OpenFile(abs, flag, perm) + } + + // Check for other valid reads. + thisDir, thisFile := path.Split(file.abs) + + // Allowed to read directory itself. + if name == thisDir || name == "." { + return os.OpenFile(thisDir, flag, perm) + } + + // Allowed to read file + // itself (at relative path). + if name == thisFile { + return os.OpenFile(abs, flag, perm) + } + } + + return nil, os.ErrPermission +} + // getExtension splits file extension from path. func getExtension(path string) string { for i := len(path) - 1; i >= 0 && path[i] != '/'; i-- {