Validate aliases to prevent directory traversal etc.

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
author: Anthony Fok <foka@debian.org> 2015-09-13 15:36:08 +0300
committer: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com> 2015-09-13 19:08:42 +0300
commit: e71bef79e504f8a016652380ad4c0ca89a2b8898 (patch)
tree: bec7ec563ccd360f4f343304f1bc100805ddd5e9 /target
parent: db85299c0039fd13fd18067236e6f3b27e167fb7 (diff)
2 files changed, 84 insertions, 19 deletions
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 = "<!DOCTYPE html><html><head><link rel=\"canonical\" href=\"{{ .Permalink }}\"/><meta http-equiv=\"content-type\" content=\"text/html; charset=utf-8\" /><meta http-equiv=\"refresh\" content=\"0;url={{ .Permalink }}\" /></head></html>"
@@ -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")
+	}
+
+	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)
 	}
 
-	if strings.HasSuffix(alias, "/") {
+	// 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"
author	Anthony Fok <foka@debian.org>	2015-09-13 15:36:08 +0300
committer	Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>	2015-09-13 19:08:42 +0300
commit	e71bef79e504f8a016652380ad4c0ca89a2b8898 (patch)
tree	bec7ec563ccd360f4f343304f1bc100805ddd5e9 /target
parent	db85299c0039fd13fd18067236e6f3b27e167fb7 (diff)