From e71bef79e504f8a016652380ad4c0ca89a2b8898 Mon Sep 17 00:00:00 2001 From: Anthony Fok Date: Sun, 13 Sep 2015 06:36:08 -0600 Subject: [PATCH] Validate aliases to prevent directory traversal etc. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add validation before creating aliases: * Prevent creating aliases outside webroot (public/ dir) * Skip empty "" alias * Skip "/" → "/index.html", which gets overwritten anyway * Refuse to create Windows-invalid filenames on Windows; warn on other platforms * In case of invalid aliases, after skipping them, return `err = nil` to prevent the error passing up all the way to `hugolib.Render()` and causing Hugo to abort. * Update alias tests. Fixes #701: Add support for alias with whitespace Fixes #1418: Add validation for alias --- hugolib/site.go | 3 +- target/alias_test.go | 38 +++++++++++++++--------- target/htmlredirect.go | 65 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 86 insertions(+), 20 deletions(-) diff --git a/hugolib/site.go b/hugolib/site.go index 3ac00d0d7..4cf31e4ab 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -242,6 +242,7 @@ func (s *Site) Build() (err error) { return } if err = s.Render(); err != nil { + // Better reporting when the template is missing (commit 2bbecc7b) jww.ERROR.Printf("Error rendering site: %s\nAvailable templates:\n", err) for _, template := range s.Tmpl.Templates() { jww.ERROR.Printf("\t%s\n", template.Name()) @@ -1566,7 +1567,7 @@ func (s *Site) WriteDestPage(path string, reader io.Reader) (err error) { } func (s *Site) WriteDestAlias(path string, permalink template.HTML) (err error) { - jww.DEBUG.Println("alias created at:", path) + jww.DEBUG.Println("creating alias:", path) return s.AliasTarget().Publish(path, permalink) } diff --git a/target/alias_test.go b/target/alias_test.go index ec686af54..13a8889dc 100644 --- a/target/alias_test.go +++ b/target/alias_test.go @@ -2,6 +2,7 @@ package target import ( "path/filepath" + "runtime" "testing" ) @@ -9,29 +10,38 @@ func TestHTMLRedirectAlias(t *testing.T) { var o Translator o = new(HTMLRedirectAlias) + errIsNilForThisOS := runtime.GOOS != "windows" + tests := []struct { value string expected string + errIsNil bool }{ - {"", ""}, - {"s", filepath.FromSlash("s/index.html")}, - {"/", filepath.FromSlash("/index.html")}, - {"alias 1", filepath.FromSlash("alias-1/index.html")}, - {"alias 2/", filepath.FromSlash("alias-2/index.html")}, - {"alias 3.html", "alias-3.html"}, - {"alias4.html", "alias4.html"}, - {"/alias 5.html", filepath.FromSlash("/alias-5.html")}, - {"/трям.html", filepath.FromSlash("/трям.html")}, + {"", "", false}, + {"s", filepath.FromSlash("s/index.html"), true}, + {"/", "", false}, + {"alias 1", filepath.FromSlash("alias 1/index.html"), true}, + {"alias 2/", filepath.FromSlash("alias 2/index.html"), true}, + {"alias 3.html", "alias 3.html", true}, + {"alias4.html", "alias4.html", true}, + {"/alias 5.html", "alias 5.html", true}, + {"/трям.html", "трям.html", true}, + {"../../../../tmp/passwd", "", false}, + {"/foo/../../../../tmp/passwd", filepath.FromSlash("tmp/passwd/index.html"), true}, + {"foo/../../../../tmp/passwd", "", false}, + {"C:\\Windows", filepath.FromSlash("C:\\Windows/index.html"), errIsNilForThisOS}, + {"/chrome/?p=help&ctx=keyboard#topic=3227046", filepath.FromSlash("chrome/?p=help&ctx=keyboard#topic=3227046/index.html"), errIsNilForThisOS}, + {"/LPT1/Printer/", filepath.FromSlash("LPT1/Printer/index.html"), errIsNilForThisOS}, } for _, test := range tests { path, err := o.Translate(test.value) - if err != nil { - t.Fatalf("Translate returned an error: %s", err) + if (err == nil) != test.errIsNil { + t.Errorf("Expected err == nil => %t, got: %t. err: %s", test.errIsNil, err == nil, err) + continue } - - if path != test.expected { - t.Errorf("Expected: %s, got: %s", test.expected, path) + if err == nil && path != test.expected { + t.Errorf("Expected: \"%s\", got: \"%s\"", test.expected, path) } } } diff --git a/target/htmlredirect.go b/target/htmlredirect.go index d2fb5f527..010428a25 100644 --- a/target/htmlredirect.go +++ b/target/htmlredirect.go @@ -2,12 +2,15 @@ package target import ( "bytes" + "fmt" "html/template" "path/filepath" + "runtime" "strings" "github.com/spf13/hugo/helpers" "github.com/spf13/hugo/hugofs" + jww "github.com/spf13/jwalterweatherman" ) const ALIAS = "" @@ -32,16 +35,67 @@ type HTMLRedirectAlias struct { } func (h *HTMLRedirectAlias) Translate(alias string) (aliasPath string, err error) { + originalAlias := alias if len(alias) <= 0 { - return + return "", fmt.Errorf("Alias \"\" is an empty string") } - if strings.HasSuffix(alias, "/") { + alias = filepath.Clean(alias) + components := strings.Split(alias, helpers.FilePathSeparator) + + if alias == helpers.FilePathSeparator { + return "", fmt.Errorf("Alias \"%s\" resolves to website root directory", originalAlias) + } + + // Validate against directory traversal + if components[0] == ".." { + return "", fmt.Errorf("Alias \"%s\" traverses outside the website root directory", originalAlias) + } + + // Handle Windows filename restrictions + msgs := []string{} + reservedNames := []string{"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"} + + if strings.ContainsAny(alias, ":*?\"<>|") { + msgs = append(msgs, fmt.Sprintf("Alias \"%s\" contains invalid characters in a filename on Windows: : * ? \" < > |", originalAlias)) + } + for _, c := range components { + if strings.HasSuffix(c, ".") { + msgs = append(msgs, fmt.Sprintf("Alias \"%s\" contains component with trailing period, invalid on Windows", originalAlias)) + } + for _, r := range reservedNames { + if c == r { + msgs = append(msgs, fmt.Sprintf("Alias \"%s\" contains component with reserved name \"%s\" on Windows", originalAlias, r)) + } + } + } + if len(msgs) > 0 { + if runtime.GOOS == "windows" { + for _, m := range msgs { + jww.ERROR.Println(m) + } + return "", fmt.Errorf("Cannot create \"%s\": Windows filename restriction", originalAlias) + } else { + for _, m := range msgs { + jww.WARN.Println(m) + } + } + } + + // Add the final touch + if strings.HasPrefix(alias, helpers.FilePathSeparator) { + alias = alias[1:] + } + if strings.HasSuffix(alias, helpers.FilePathSeparator) { alias = alias + "index.html" } else if !strings.HasSuffix(alias, ".html") { - alias = alias + "/index.html" + alias = alias + helpers.FilePathSeparator + "index.html" } - return filepath.Join(h.PublishDir, helpers.MakePath(alias)), nil + if originalAlias != alias { + jww.INFO.Printf("Alias \"%s\" translated to \"%s\"\n", originalAlias, alias) + } + + return filepath.Join(h.PublishDir, alias), nil } type AliasNode struct { @@ -50,7 +104,8 @@ type AliasNode struct { func (h *HTMLRedirectAlias) Publish(path string, permalink template.HTML) (err error) { if path, err = h.Translate(path); err != nil { - return + jww.ERROR.Printf("%s, skipping.", err) + return nil } t := "alias"