zap.Open: Invalidate relative paths and paths with ".." segments

Currently, zap.Open doesn't prevent someone from explicitly doing something
like zap.Open("file://../../../secret").

zap.Open already documents that paths passed to it must be absolute. Add
validation to error if zap.Open is called with a relative paths that could
write files outside of intended file directory hierarchy.

This change addresses https://cwe.mitre.org/data/definitions/23.html

ref #1390

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.

writer_test.go

@@ -224,6 +224,46 @@ func TestOpenOtherErrors(t *testing.T) {
  }
 }
 
+func TestOpenValidation(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