fileserver: reject short names in every path component (#7952)

* fileserver: reject short names in every path component
Signed-off-by: DavidCarliez <271374756+DavidCarliez@users.noreply.github.com>

* fileserver: validate short-name characters
Signed-off-by: DavidCarliez <271374756+DavidCarliez@users.noreply.github.com>

* fileserver: fail closed on extended short names
Signed-off-by: DavidCarliez <271374756+DavidCarliez@users.noreply.github.com>

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

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) {

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)
+			}
+		})
+	}
+}