From 3436595715aa1a02f9942d5eead1d2c3995d578e Mon Sep 17 00:00:00 2001
From: Ryan Hang <rhang@uber.com>
Date: Sat, 16 Dec 2023 15:10:37 -0800
Subject: [PATCH] zap.Open: Invalidate relative path roots

Add validation to ensure file schema paths passed to zap.Open are
absolute since this is already documented.

Tests are also added to demonstrate that double dot segements within
file schema URIs passed to zap.Open remaining within the specified file
hierarchy.

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

ref https://github.com/uber-go/zap/issues/1390
---
 sink.go        |  5 ++++
 writer_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/sink.go b/sink.go
index 499772a00..85ce697f7 100644
--- a/sink.go
+++ b/sink.go
@@ -103,6 +103,11 @@ func (sr *sinkRegistry) newSink(rawURL string) (Sink, error) {
 	if err != nil {
 		return nil, fmt.Errorf("can't parse %q as a URL: %v", rawURL, err)
 	}
+
+	if u.Scheme == schemeFile && !filepath.IsAbs(u.Path) {
+		return nil, fmt.Errorf("file URI %q attempts a relative path", rawURL)
+	}
+
 	if u.Scheme == "" {
 		u.Scheme = schemeFile
 	}
diff --git a/writer_test.go b/writer_test.go
index 20e00b74b..4cce8edb0 100644
--- a/writer_test.go
+++ b/writer_test.go
@@ -224,6 +224,85 @@ func TestOpenOtherErrors(t *testing.T) {
 	}
 }
 
+func TestOpenRelativeValidated(t *testing.T) {
+	tests := []struct {
+		msg     string
+		paths   []string
+		wantErr string
+	}{
+		{
+			msg: "invalid relative path root",
+			paths: []string{
+				"file:../some/path",
+			},
+			// url.Parse's Path for this path value is "" which would result
+			// in a file not found error if not validated.
+			wantErr: `open sink "file:../some/path": file URI "file:../some/path" attempts a relative path`,
+		},
+		{
+			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)
+		})
+	}
+}
+
+func TestOpenDotSegmentsSanitized(t *testing.T) {
+	tempName := filepath.Join(t.TempDir(), "test.log")
+	assert.False(t, fileExists(tempName))
+	require.True(t, filepath.IsAbs(tempName), "Expected absolute temp file path.")
+
+	tests := []struct {
+		paths            []string
+		toWrite          []byte
+		wantFileContents string
+	}{
+		{
+			paths:            []string{"file:/.." + tempName},
+			toWrite:          []byte("a"),
+			wantFileContents: "a",
+		},
+		{
+			paths:            []string{"file:/../.." + tempName},
+			toWrite:          []byte("b"),
+			wantFileContents: "ab",
+		},
+		{
+			paths:            []string{"file:///.." + tempName},
+			toWrite:          []byte("c"),
+			wantFileContents: "abc",
+		},
+		{
+			paths:            []string{"file:///../.." + tempName},
+			toWrite:          []byte("d"),
+			wantFileContents: "abcd",
+		},
+	}
+
+	for _, tt := range tests {
+		ws, cleanup, err := Open(tt.paths...)
+		require.NoError(t, err)
+		defer cleanup()
+
+		_, err = ws.Write(tt.toWrite)
+		require.NoError(t, err)
+
+		b, err := os.ReadFile(tempName)
+		require.NoError(t, err)
+
+		assert.Equal(t, string(b), tt.wantFileContents)
+	}
+}
+
 type testWriter struct {
 	expected string
 	t        testing.TB