@marcel91200 / caddy Mirror · read-only / commits / 9d4ed3a3
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 14, 05:15 UTC

caddyhttp: Refactor and export SanitizedPathJoin for use in fastcgi (#4207)

Matt Holt committed Jun 17, 2021 at 09:59 UTC 9d4ed3a3236df06e54c80c4f6633b66d68ad3673
8 files changed +131 -134
modules/caddyhttp/caddyhttp.go
+29
@@ -20,7 +20,9 @@ import (
20 "io"
21 "net"
22 "net/http"
23 + "path/filepath"
24 "strconv"
25 + "strings"
26
27 "github.com/caddyserver/caddy/v2"
28 "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
@@ -217,6 +219,31 @@ func StatusCodeMatches(actual, configured int) bool {
219 return false
220 }
221
222 +// SanitizedPathJoin performs filepath.Join(root, reqPath) that
223 +// is safe against directory traversal attacks. It uses logic
224 +// similar to that in the Go standard library, specifically
225 +// in the implementation of http.Dir. The root is assumed to
226 +// be a trusted path, but reqPath is not; and the output will
227 +// never be outside of root. The resulting path can be used
228 +// with the local file system.
229 +func SanitizedPathJoin(root, reqPath string) string {
230 + if root == "" {
231 + root = "."
232 + }
233 +
234 + path := filepath.Join(root, filepath.Clean("/"+reqPath))
235 +
236 + // filepath.Join also cleans the path, and cleaning strips
237 + // the trailing slash, so we need to re-add it afterwards.
238 + // if the length is 1, then it's a path to the root,
239 + // and that should return ".", so we don't append the separator.
240 + if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 {
241 + path += separator
242 + }
243 +
244 + return path
245 +}
246 +
247 // tlsPlaceholderWrapper is a no-op listener wrapper that marks
248 // where the TLS listener should be in a chain of listener wrappers.
249 // It should only be used if another listener wrapper must be placed
@@ -242,6 +269,8 @@ const (
269 DefaultHTTPSPort = 443
270 )
271
272 +const separator = string(filepath.Separator)
273 +
274 // Interface guard
275 var _ caddy.ListenerWrapper = (*tlsPlaceholderWrapper)(nil)
276 var _ caddyfile.Unmarshaler = (*tlsPlaceholderWrapper)(nil)
modules/caddyhttp/caddyhttp_test.go new
+94
@@ -0,0 +1,94 @@
1 +package caddyhttp
2 +
3 +import (
4 + "net/url"
5 + "path/filepath"
6 + "testing"
7 +)
8 +
9 +func TestSanitizedPathJoin(t *testing.T) {
10 + // For reference:
11 + // %2e = .
12 + // %2f = /
13 + // %5c = \
14 + for i, tc := range []struct {
15 + inputRoot string
16 + inputPath string
17 + expect string
18 + }{
19 + {
20 + inputPath: "",
21 + expect: ".",
22 + },
23 + {
24 + inputPath: "/",
25 + expect: ".",
26 + },
27 + {
28 + inputPath: "/foo",
29 + expect: "foo",
30 + },
31 + {
32 + inputPath: "/foo/",
33 + expect: "foo" + separator,
34 + },
35 + {
36 + inputPath: "/foo/bar",
37 + expect: filepath.Join("foo", "bar"),
38 + },
39 + {
40 + inputRoot: "/a",
41 + inputPath: "/foo/bar",
42 + expect: filepath.Join("/", "a", "foo", "bar"),
43 + },
44 + {
45 + inputPath: "/foo/../bar",
46 + expect: "bar",
47 + },
48 + {
49 + inputRoot: "/a/b",
50 + inputPath: "/foo/../bar",
51 + expect: filepath.Join("/", "a", "b", "bar"),
52 + },
53 + {
54 + inputRoot: "/a/b",
55 + inputPath: "/..%2fbar",
56 + expect: filepath.Join("/", "a", "b", "bar"),
57 + },
58 + {
59 + inputRoot: "/a/b",
60 + inputPath: "/%2e%2e%2fbar",
61 + expect: filepath.Join("/", "a", "b", "bar"),
62 + },
63 + {
64 + inputRoot: "/a/b",
65 + inputPath: "/%2e%2e%2f%2e%2e%2f",
66 + expect: filepath.Join("/", "a", "b") + separator,
67 + },
68 + {
69 + inputRoot: "C:\\www",
70 + inputPath: "/foo/bar",
71 + expect: filepath.Join("C:\\www", "foo", "bar"),
72 + },
73 + {
74 + inputRoot: "C:\\www",
75 + inputPath: "/D:\\foo\\bar",
76 + expect: filepath.Join("C:\\www", "D:\\foo\\bar"),
77 + },
78 + } {
79 + // we don't *need* to use an actual parsed URL, but it
80 + // adds some authenticity to the tests since real-world
81 + // values will be coming in from URLs; thus, the test
82 + // corpus can contain paths as encoded by clients, which
83 + // more closely emulates the actual attack vector
84 + u, err := url.Parse("http://test:9999" + tc.inputPath)
85 + if err != nil {
86 + t.Fatalf("Test %d: invalid URL: %v", i, err)
87 + }
88 + actual := SanitizedPathJoin(tc.inputRoot, u.Path)
89 + if actual != tc.expect {
90 + t.Errorf("Test %d: SanitizedPathJoin('%s', '%s') => %s (expected '%s')",
91 + i, tc.inputRoot, tc.inputPath, actual, tc.expect)
92 + }
93 + }
94 +}
modules/caddyhttp/fileserver/browse.go
+1 -1
@@ -207,7 +207,7 @@ func isSymlinkTargetDir(f os.FileInfo, root, urlPath string) bool {
207 if !isSymlink(f) {
208 return false
209 }
210 - target := sanitizedPathJoin(root, path.Join(urlPath, f.Name()))
210 + target := caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, f.Name()))
211 targetInfo, err := os.Stat(target)
212 if err != nil {
213 return false
modules/caddyhttp/fileserver/matcher.go
+1 -1
@@ -185,7 +185,7 @@ func (m MatchFile) selectFile(r *http.Request) (matched bool) {
185 if strings.HasSuffix(file, "/") {
186 suffix += "/"
187 }
188 - fullpath = sanitizedPathJoin(root, suffix)
188 + fullpath = caddyhttp.SanitizedPathJoin(root, suffix)
189 return
190 }
191
modules/caddyhttp/fileserver/matcher_test.go
+2 -2
@@ -94,7 +94,7 @@ func TestFileMatcher(t *testing.T) {
94 t.Fatalf("Test %d: actual path: %v, expected: %v", i, rel, tc.expectedPath)
95 }
96
97 - fileType, ok := repl.Get("http.matchers.file.type")
97 + fileType, _ := repl.Get("http.matchers.file.type")
98 if fileType != tc.expectedType {
99 t.Fatalf("Test %d: actual file type: %v, expected: %v", i, fileType, tc.expectedType)
100 }
@@ -197,7 +197,7 @@ func TestPHPFileMatcher(t *testing.T) {
197 t.Fatalf("Test %d: actual path: %v, expected: %v", i, rel, tc.expectedPath)
198 }
199
200 - fileType, ok := repl.Get("http.matchers.file.type")
200 + fileType, _ := repl.Get("http.matchers.file.type")
201 if fileType != tc.expectedType {
202 t.Fatalf("Test %d: actual file type: %v, expected: %v", i, fileType, tc.expectedType)
203 }
modules/caddyhttp/fileserver/staticfiles.go
+2 -38
@@ -161,7 +161,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
161 filesToHide := fsrv.transformHidePaths(repl)
162
163 root := repl.ReplaceAll(fsrv.Root, ".")
164 - filename := sanitizedPathJoin(root, r.URL.Path)
164 + filename := caddyhttp.SanitizedPathJoin(root, r.URL.Path)
165
166 fsrv.logger.Debug("sanitized path join",
167 zap.String("site_root", root),
@@ -185,7 +185,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
185 var implicitIndexFile bool
186 if info.IsDir() && len(fsrv.IndexNames) > 0 {
187 for _, indexPage := range fsrv.IndexNames {
188 - indexPath := sanitizedPathJoin(filename, indexPage)
188 + indexPath := caddyhttp.SanitizedPathJoin(filename, indexPage)
189 if fileHidden(indexPath, filesToHide) {
190 // pretend this file doesn't exist
191 fsrv.logger.Debug("hiding index file",
@@ -435,42 +435,6 @@ func (fsrv *FileServer) transformHidePaths(repl *caddy.Replacer) []string {
435 return hide
436 }
437
438 -// sanitizedPathJoin performs filepath.Join(root, reqPath) that
439 -// is safe against directory traversal attacks. It uses logic
440 -// similar to that in the Go standard library, specifically
441 -// in the implementation of http.Dir. The root is assumed to
442 -// be a trusted path, but reqPath is not.
443 -func sanitizedPathJoin(root, reqPath string) string {
444 - // TODO: Caddy 1 uses this:
445 - // prevent absolute path access on Windows, e.g. http://localhost:5000/C:\Windows\notepad.exe
446 - // if runtime.GOOS == "windows" && len(reqPath) > 0 && filepath.IsAbs(reqPath[1:]) {
447 - // TODO.
448 - // }
449 -
450 - // TODO: whereas std lib's http.Dir.Open() uses this:
451 - // if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) {
452 - // return nil, errors.New("http: invalid character in file path")
453 - // }
454 -
455 - // TODO: see https://play.golang.org/p/oh77BiVQFti for another thing to consider
456 -
457 - if root == "" {
458 - root = "."
459 - }
460 -
461 - path := filepath.Join(root, filepath.Clean("/"+reqPath))
462 -
463 - // filepath.Join also cleans the path, and cleaning strips
464 - // the trailing slash, so we need to re-add it afterwards.
465 - // if the length is 1, then it's a path to the root,
466 - // and that should return ".", so we don't append the separator.
467 - if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 {
468 - path += separator
469 - }
470 -
471 - return path
472 -}
473 -
438 // fileHidden returns true if filename is hidden according to the hide list.
439 // filename must be a relative or absolute file system path, not a request
440 // URI path. It is expected that all the paths in the hide list are absolute
modules/caddyhttp/fileserver/staticfiles_test.go
-84
@@ -15,96 +15,12 @@
15 package fileserver
16
17 import (
18 - "net/url"
18 "path/filepath"
19 "runtime"
20 "strings"
21 "testing"
22 )
23
25 -func TestSanitizedPathJoin(t *testing.T) {
26 - // For easy reference:
27 - // %2e = .
28 - // %2f = /
29 - // %5c = \
30 - for i, tc := range []struct {
31 - inputRoot string
32 - inputPath string
33 - expect string
34 - }{
35 - {
36 - inputPath: "",
37 - expect: ".",
38 - },
39 - {
40 - inputPath: "/",
41 - expect: ".",
42 - },
43 - {
44 - inputPath: "/foo",
45 - expect: "foo",
46 - },
47 - {
48 - inputPath: "/foo/",
49 - expect: "foo" + separator,
50 - },
51 - {
52 - inputPath: "/foo/bar",
53 - expect: filepath.Join("foo", "bar"),
54 - },
55 - {
56 - inputRoot: "/a",
57 - inputPath: "/foo/bar",
58 - expect: filepath.Join("/", "a", "foo", "bar"),
59 - },
60 - {
61 - inputPath: "/foo/../bar",
62 - expect: "bar",
63 - },
64 - {
65 - inputRoot: "/a/b",
66 - inputPath: "/foo/../bar",
67 - expect: filepath.Join("/", "a", "b", "bar"),
68 - },
69 - {
70 - inputRoot: "/a/b",
71 - inputPath: "/..%2fbar",
72 - expect: filepath.Join("/", "a", "b", "bar"),
73 - },
74 - {
75 - inputRoot: "/a/b",
76 - inputPath: "/%2e%2e%2fbar",
77 - expect: filepath.Join("/", "a", "b", "bar"),
78 - },
79 - {
80 - inputRoot: "/a/b",
81 - inputPath: "/%2e%2e%2f%2e%2e%2f",
82 - expect: filepath.Join("/", "a", "b") + separator,
83 - },
84 - {
85 - inputRoot: "C:\\www",
86 - inputPath: "/foo/bar",
87 - expect: filepath.Join("C:\\www", "foo", "bar"),
88 - },
89 - // TODO: test more windows paths... on windows... sigh.
90 - } {
91 - // we don't *need* to use an actual parsed URL, but it
92 - // adds some authenticity to the tests since real-world
93 - // values will be coming in from URLs; thus, the test
94 - // corpus can contain paths as encoded by clients, which
95 - // more closely emulates the actual attack vector
96 - u, err := url.Parse("http://test:9999" + tc.inputPath)
97 - if err != nil {
98 - t.Fatalf("Test %d: invalid URL: %v", i, err)
99 - }
100 - actual := sanitizedPathJoin(tc.inputRoot, u.Path)
101 - if actual != tc.expect {
102 - t.Errorf("Test %d: [%s %s] => %s (expected %s)",
103 - i, tc.inputRoot, tc.inputPath, actual, tc.expect)
104 - }
105 - }
106 -}
107 -
24 func TestFileHidden(t *testing.T) {
25 for i, tc := range []struct {
26 inputHide []string
modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
+2 -8
@@ -20,7 +20,6 @@ import (
20 "fmt"
21 "net"
22 "net/http"
23 - "path"
23 "path/filepath"
24 "strconv"
25 "strings"
@@ -218,12 +217,7 @@ func (t Transport) buildEnv(r *http.Request) (map[string]string, error) {
217 }
218
219 // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
221 - scriptFilename := filepath.Join(root, scriptName)
222 -
223 - // Add vhost path prefix to scriptName. Otherwise, some PHP software will
224 - // have difficulty discovering its URL.
225 - pathPrefix, _ := r.Context().Value(caddy.CtxKey("path_prefix")).(string)
226 - scriptName = path.Join(pathPrefix, scriptName)
220 + scriptFilename := caddyhttp.SanitizedPathJoin(root, scriptName)
221
222 // Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
223 // Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
@@ -288,7 +282,7 @@ func (t Transport) buildEnv(r *http.Request) (map[string]string, error) {
282 // PATH_TRANSLATED should only exist if PATH_INFO is defined.
283 // Info: https://www.ietf.org/rfc/rfc3875 Page 14
284 if env["PATH_INFO"] != "" {
291 - env["PATH_TRANSLATED"] = filepath.Join(root, pathInfo) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html
285 + env["PATH_TRANSLATED"] = caddyhttp.SanitizedPathJoin(root, pathInfo) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html
286 }
287
288 // compliance with the CGI specification requires that