diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index bd9df8223..fa272de06 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -30,6 +30,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -266,6 +267,57 @@ func (fsrv *FileServer) Provision(ctx caddy.Context) error { return nil } +const windowsShortNamePunctuation = "$%'-_@~`!(){}^#&" + +// possibleWindowsShortNamePart reports whether s could be part of an 8.3 +// alias. Extended characters are allowed when Windows' allowextchar behavior +// is enabled, so non-ASCII characters must be accepted here to fail closed. +func possibleWindowsShortNamePart(s string) bool { + for _, c := range s { + if c >= utf8.RuneSelf || + (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || + strings.ContainsRune(windowsShortNamePunctuation, c) { + continue + } + return false + } + return true +} + +// hasWindowsShortName reports whether any component of p has the syntax of an +// NTFS 8.3 short name. Both slash types separate components on Windows, which +// also ignores trailing dots and spaces in each component. +func hasWindowsShortName(p string) bool { + for _, component := range strings.FieldsFunc(p, func(r rune) bool { + return r == '/' || r == '\\' + }) { + component = strings.TrimRight(component, ". ") + base, extension, hasExtension := strings.Cut(component, ".") + if utf8.RuneCountInString(base) > 8 || !possibleWindowsShortNamePart(base) || + (hasExtension && (utf8.RuneCountInString(extension) > 3 || !possibleWindowsShortNamePart(extension))) { + continue + } + + tilde := strings.LastIndexByte(base, '~') + if tilde <= 0 || tilde == len(base)-1 { + continue + } + shortNameNumber := base[tilde+1:] + for _, c := range shortNameNumber { + if c < '0' || c > '9' { + shortNameNumber = "" + break + } + } + if shortNameNumber != "" { + return true + } + } + return false +} + func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) @@ -275,8 +327,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c return caddyhttp.Error(http.StatusBadRequest, fmt.Errorf("illegal ADS path")) } // reject paths with "8.3" short names - trimmedPath := strings.TrimRight(r.URL.Path, ". ") // Windows ignores trailing dots and spaces, sigh - if len(path.Base(trimmedPath)) <= 12 && strings.Contains(trimmedPath, "~") { + if hasWindowsShortName(r.URL.Path) { return caddyhttp.Error(http.StatusBadRequest, fmt.Errorf("illegal short name")) } // both of those could bypass file hiding or possibly leak information even if the file is not hidden diff --git a/modules/caddyhttp/fileserver/staticfiles_test.go b/modules/caddyhttp/fileserver/staticfiles_test.go index a077a2011..a588dcb2b 100644 --- a/modules/caddyhttp/fileserver/staticfiles_test.go +++ b/modules/caddyhttp/fileserver/staticfiles_test.go @@ -140,6 +140,111 @@ func TestFileHidden(t *testing.T) { } } +func TestHasWindowsShortName(t *testing.T) { + for _, tc := range []struct { + name string + path string + want bool + }{ + { + name: "final component", + path: "/DIRECT~1.TXT", + want: true, + }, + { + name: "parent component with long final filename", + path: "/PROTEC~1/this-is-a-long-final-filename.txt", + want: true, + }, + { + name: "middle component", + path: "/public/PROTEC~1/file.txt", + want: true, + }, + { + name: "backslash separator", + path: `\public\PROTEC~1\file.txt`, + want: true, + }, + { + name: "two digit short name suffix", + path: "/PROTE~12/file.txt", + want: true, + }, + { + name: "trailing dots and spaces", + path: "/PROTEC~1. /file.txt", + want: true, + }, + { + name: "permitted punctuation", + path: "/$%'-_~1.!#&", + want: true, + }, + { + name: "ordinary tilde name", + path: "/ordinary~name/file.txt", + want: false, + }, + { + name: "space in short name shape", + path: "/my f~1.txt", + want: false, + }, + { + name: "space in long tilde name", + path: "/my file~1.txt", + want: false, + }, + { + name: "non-ASCII short name shape in parent component", + path: "/public/café~1/this-is-a-long-final-filename.txt", + want: true, + }, + { + name: "non-ASCII ordinary tilde name", + path: "/café~name.txt", + want: false, + }, + { + name: "forbidden punctuation", + path: "/MY+F~1.TXT", + want: false, + }, + { + name: "tilde followed by non-digit", + path: "/SHORT~A/file.txt", + want: false, + }, + { + name: "characters after numeric suffix", + path: "/SHORT~1-file.txt", + want: false, + }, + { + name: "base too long for 8.3", + path: "/TOOLONG~1/file.txt", + want: false, + }, + { + name: "extension too long for 8.3", + path: "/SHORT~1.LONG/file.txt", + want: false, + }, + { + name: "multiple extension separators", + path: "/SHORT~1.T.X/file.txt", + want: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := hasWindowsShortName(tc.path); got != tc.want { + t.Fatalf("hasWindowsShortName(%q) = %t, want %t", tc.path, got, tc.want) + } + }) + } +} + // Check to make sure that we don't serve ETag and Last-Modified headers // for files with invalid modification times func TestModTimeHeaders(t *testing.T) { diff --git a/modules/caddyhttp/fileserver/staticfiles_windows_test.go b/modules/caddyhttp/fileserver/staticfiles_windows_test.go new file mode 100644 index 000000000..b940bde90 --- /dev/null +++ b/modules/caddyhttp/fileserver/staticfiles_windows_test.go @@ -0,0 +1,88 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fileserver + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +func TestFileServerRejectsShortNameInParentComponent(t *testing.T) { + root := t.TempDir() + ordinaryNames := []string{ + "ordinary~name-with-long-suffix.txt", + "my f~1.txt", + "my file~1.txt", + "café~name.txt", + } + for _, name := range ordinaryNames { + if err := os.WriteFile(filepath.Join(root, name), []byte(name), 0o600); err != nil { + t.Fatal(err) + } + } + + fsrv := FileServer{ + Root: root, + CanonicalURIs: new(bool), + } + ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) + if err := fsrv.Provision(ctx); err != nil { + t.Fatal(err) + } + + for _, requestPath := range []string{ + "/PROTEC~1/this-is-a-long-final-filename.txt", + "/public/caf%C3%A9~1/this-is-a-long-final-filename.txt", + } { + t.Run(requestPath, func(t *testing.T) { + err := fsrv.ServeHTTP( + httptest.NewRecorder(), + newPrecompressedRequest(t, requestPath), + nil, + ) + var handlerErr caddyhttp.HandlerError + if !errors.As(err, &handlerErr) { + t.Fatalf("expected HandlerError, got %v", err) + } + if handlerErr.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", handlerErr.StatusCode, http.StatusBadRequest) + } + }) + } + + for _, name := range ordinaryNames { + t.Run(name, func(t *testing.T) { + w := httptest.NewRecorder() + if err := fsrv.ServeHTTP(w, newPrecompressedRequest(t, "/"+url.PathEscape(name)), nil); err != nil { + t.Fatal(err) + } + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) + } + if got := w.Body.String(); got != name { + t.Fatalf("body = %q, want %q", got, name) + } + }) + } +}