diff --git a/writer.go b/writer.go index 06768c679..b30a9ada6 100644 --- a/writer.go +++ b/writer.go @@ -23,6 +23,9 @@ package zap import ( "fmt" "io" + "net/url" + "path/filepath" + "strings" "go.uber.org/zap/zapcore" @@ -48,6 +51,10 @@ import ( // os.Stdout and os.Stderr. When specified without a scheme, relative file // paths also work. func Open(paths ...string) (zapcore.WriteSyncer, func(), error) { + if err := checkOpenPaths(paths); err != nil { + return nil, nil, err + } + writers, closeAll, err := open(paths) if err != nil { return nil, nil, err @@ -84,6 +91,23 @@ func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { return writers, closeAll, nil } +func checkOpenPaths(paths []string) error { + var openErr error + for _, path := range paths { + if !strings.HasPrefix(path, "file:") { + continue + } + u, err := url.Parse(path) + if err != nil { + return err + } + if !(filepath.IsAbs(u.Path) && !strings.Contains(u.Path, "..")) { + openErr = multierr.Append(openErr, fmt.Errorf(`file URI %q attempts a relative path or contains ".." dot segments`, path)) + } + } + return openErr +} + // CombineWriteSyncers is a utility that combines multiple WriteSyncers into a // single, locked WriteSyncer. If no inputs are supplied, it returns a no-op // WriteSyncer. diff --git a/writer_test.go b/writer_test.go index 20e00b74b..c18222d07 100644 --- a/writer_test.go +++ b/writer_test.go @@ -224,6 +224,46 @@ func TestOpenOtherErrors(t *testing.T) { } } +func TestOpenPathTraversalValidation(t *testing.T) { + tests := []struct { + msg string + paths []string + wantErr string + }{ + { + msg: "invalid relative path root", + paths: []string{ + "file:/../some/path", + "file:///../some/path", + }, + wantErr: `file URI "file:/../some/path" attempts a relative path or contains ".." dot segments; file URI "file:///../some/path" attempts a relative path or contains ".." dot segments`, + }, + { + msg: "invalid absolute path root with double dot segments", + paths: []string{ + "file:/some/../../path", + "file://some/../../path", + "file:///some/../../path", + }, + wantErr: `file URI "file:/some/../../path" attempts a relative path or contains ".." dot segments; file URI "file://some/../../path" attempts a relative path or contains ".." dot segments; file URI "file:///some/../../path" attempts a relative path or contains ".." dot segments`, + }, + { + msg: "invalid double dot as the host element", + paths: []string{ + "file://../some/path", + }, + wantErr: `open sink "file://../some/path": file URLs must leave host empty or use localhost: got file://../some/path`, + }, + } + + for _, tt := range tests { + t.Run(tt.msg, func(t *testing.T) { + _, _, err := Open(tt.paths...) + assert.EqualError(t, err, tt.wantErr) + }) + } +} + type testWriter struct { expected string t testing.TB