diff --git a/README.md b/README.md index fb7e184..49750cf 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,6 @@ To view more specific examples, you could visit each engine folder to learn more - [pug](./pug/README.md) - [slim](./slim/README.md) - ### embedded Systems We support the `http.FileSystem` interface, so you can use different libraries to load the templates from embedded binaries. diff --git a/ace/README.md b/ace/README.md index 9c0d07c..d10ac61 100644 --- a/ace/README.md +++ b/ace/README.md @@ -82,7 +82,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Ace templates in this repository compile down to Go's `html/template`, so output benefits from contextual escaping by default. +- Layout composition should continue to use `{{embed}}`, which receives already-rendered child output. +- Custom helpers become a trust boundary when they return trusted `html/template` types such as `template.HTML`, `template.JS`, `template.URL`, or `template.CSS`. +- Do not convert user input into trusted `html/template` types unless you have already fully sanitized it for the target context. diff --git a/ace/security_test.go b/ace/security_test.go new file mode 100644 index 0000000..ba446fd --- /dev/null +++ b/ace/security_test.go @@ -0,0 +1,50 @@ +package ace + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".ace") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".ace") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} diff --git a/amber/README.md b/amber/README.md index 8eb0c24..8f9b0cc 100644 --- a/amber/README.md +++ b/amber/README.md @@ -82,7 +82,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Amber templates in this repository compile down to Go's `html/template`, so output benefits from contextual escaping by default. +- Layout composition should continue to use `#{embed()}`, which receives already-rendered child output. +- Custom helpers become a trust boundary when they return trusted `html/template` types such as `template.HTML`, `template.JS`, `template.URL`, or `template.CSS`. +- Do not convert user input into trusted `html/template` types unless you have already fully sanitized it for the target context. diff --git a/amber/amber.go b/amber/amber.go index 557955e..0e9924c 100644 --- a/amber/amber.go +++ b/amber/amber.go @@ -10,11 +10,14 @@ import ( "os" "path/filepath" "strings" + "sync" "github.com/eknkc/amber" core "github.com/gofiber/template/v2" ) +var amberFuncMapMu sync.Mutex + // Engine struct type Engine struct { core.Engine @@ -67,7 +70,14 @@ func (e *Engine) Load() error { // prepare the global amber funcs funcs := template.FuncMap{} - for k, v := range amber.FuncMap { // add the amber's default funcs + amberFuncMapMu.Lock() + previousFuncMap := amber.FuncMap + defer func() { + amber.FuncMap = previousFuncMap //nolint:reassign // restore the prior compiler state after loading templates. + amberFuncMapMu.Unlock() + }() + + for k, v := range previousFuncMap { // add the amber's default funcs funcs[k] = v } @@ -75,7 +85,7 @@ func (e *Engine) Load() error { funcs[k] = v } - amber.FuncMap = funcs //nolint:reassign // this is fine, as long as it's not run in parallel in a test. + amber.FuncMap = funcs //nolint:reassign // amber compiler reads from this package global during compilation. // Loop trough each directory and register template files walkFn := func(path string, info os.FileInfo, err error) error { diff --git a/amber/security_test.go b/amber/security_test.go new file mode 100644 index 0000000..fcb551f --- /dev/null +++ b/amber/security_test.go @@ -0,0 +1,50 @@ +package amber + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".amber") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".amber") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} diff --git a/django/README.md b/django/README.md index 476fa69..fbaab1e 100644 --- a/django/README.md +++ b/django/README.md @@ -242,3 +242,10 @@ engine.SetAutoEscape(false) Disabling auto-escape should be approached with caution. It can expose your application to XSS attacks, where malicious scripts are injected into web pages. Without auto-escaping, there is a risk of rendering harmful HTML or JavaScript from user-supplied data. It is advisable to keep auto-escape enabled unless there is a strong reason to disable it. If you do disable it, ensure all user-supplied content is thoroughly sanitized and validated to avoid XSS vulnerabilities. + +## Security + +- Auto-escaping is enabled by default and should remain enabled for untrusted content. +- `SetAutoEscape(false)`, `{% autoescape off %}`, and `safe`-style constructs disable escaping and should only be used with trusted, pre-sanitized content. +- Layout composition uses `{{embed}}`, but the repository only marks embed output as safe after the child template has already been rendered. +- Custom globals and helper functions are trusted code and should not manufacture safe HTML from untrusted input. diff --git a/django/django.go b/django/django.go index 914e942..3b8b26e 100644 --- a/django/django.go +++ b/django/django.go @@ -9,11 +9,20 @@ import ( "path/filepath" "reflect" "strings" + "sync" "github.com/flosch/pongo2/v6" core "github.com/gofiber/template/v2" ) +var ( + // pongo2 stores autoescape in a package-global variable with a default of true. + // Serialize render-time updates so separate engine instances do not race while + // temporarily swapping that shared setting for execution. + pongo2AutoescapeMu sync.Mutex + pongo2CurrentAutoEscape = true +) + // Engine struct type Engine struct { core.Engine @@ -84,8 +93,6 @@ func (e *Engine) Load() error { pongoset := pongo2.NewSet("default", pongoloader) // Set template settings pongoset.Globals.Update(e.Funcmap) - // Set autoescaping - pongo2.SetAutoescape(e.autoEscape) // Loop trough each Directory and register template files walkFn := func(path string, info os.FileInfo, err error) error { @@ -240,6 +247,16 @@ func (e *Engine) Render(out io.Writer, name string, binding interface{}, layout e.Mutex.Lock() defer e.Mutex.Unlock() + pongo2AutoescapeMu.Lock() + previousAutoEscape := pongo2CurrentAutoEscape + defer func() { + pongo2CurrentAutoEscape = previousAutoEscape + pongo2.SetAutoescape(previousAutoEscape) + pongo2AutoescapeMu.Unlock() + }() + pongo2CurrentAutoEscape = e.autoEscape + pongo2.SetAutoescape(e.autoEscape) + bind := getPongoBinding(binding) parsed, err := tmpl.Execute(bind) if err != nil { diff --git a/django/security_test.go b/django/security_test.go new file mode 100644 index 0000000..6c5cce4 --- /dev/null +++ b/django/security_test.go @@ -0,0 +1,82 @@ +package django + +import ( + "bytes" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS_DefaultAutoEscape(t *testing.T) { + t.Parallel() + + engine := New("./views", ".django") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + + engine := New("./views", ".django") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_HelperOutputIsEscaped(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp(".", "") + require.NoError(t, err) + + defer func() { + err := os.RemoveAll(dir) + require.NoError(t, err) + }() + + err = os.WriteFile(dir+"/helper.django", []byte(`
{{ helper() }}
`), 0o600) + require.NoError(t, err) + + engine := New(dir, ".django") + engine.AddFunc("helper", func() string { + return xssPayload + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err = engine.Render(&buf, "helper", nil) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} diff --git a/handlebars/README.md b/handlebars/README.md index f01418d..33ef7ba 100644 --- a/handlebars/README.md +++ b/handlebars/README.md @@ -88,7 +88,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Handlebars escapes HTML output by default when you use `{{value}}`. +- Triple-stash expressions such as `{{{value}}}` disable escaping and should only be used for trusted, pre-sanitized markup. +- Layout composition should continue to use `{{embed}}`, which is populated from already-rendered child output. +- Helpers should return plain strings by default; returning `raymond.SafeString` should be reserved for trusted markup only. diff --git a/handlebars/security_test.go b/handlebars/security_test.go new file mode 100644 index 0000000..d56c519 --- /dev/null +++ b/handlebars/security_test.go @@ -0,0 +1,73 @@ +package handlebars + +import ( + "bytes" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".hbs") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", customMap{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".hbs") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", customMap{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_HelperOutputIsEscaped(t *testing.T) { + t.Parallel() + dir, err := os.MkdirTemp(".", "") + require.NoError(t, err) + + defer func() { + err := os.RemoveAll(dir) + require.NoError(t, err) + }() + + err = os.WriteFile(dir+"/helper.hbs", []byte(`{{helper}}
`), 0o600) + require.NoError(t, err) + + engine := New(dir, ".hbs") + engine.AddFunc("helper", func() string { + return xssPayload + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err = engine.Render(&buf, "helper", nil) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} diff --git a/html/README.md b/html/README.md index bdd6044..0076b61 100644 --- a/html/README.md +++ b/html/README.md @@ -106,11 +106,18 @@ func main() { }, "layouts/nested/main", "layouts/nested/base") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` +## Security + +- Escaping is provided by Go's `html/template`, including contextual escaping for HTML, attributes, URLs, and JavaScript-adjacent output. +- Layout composition should continue to use `{{embed}}`, which receives already-rendered child output. +- Custom helpers become a trust boundary when they return trusted `html/template` types such as `template.HTML`, `template.JS`, `template.URL`, or `template.CSS`. +- Do not convert user input into trusted `html/template` types unless you have already fully sanitized it for the target context. + ### Example with embed.FS ```go diff --git a/html/security_test.go b/html/security_test.go new file mode 100644 index 0000000..ca324aa --- /dev/null +++ b/html/security_test.go @@ -0,0 +1,86 @@ +package html + +import ( + "bytes" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".html") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".html") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_ContextualEscaping(t *testing.T) { + t.Parallel() + dir, err := os.MkdirTemp(".", "") + require.NoError(t, err) + + defer func() { + err := os.RemoveAll(dir) + require.NoError(t, err) + }() + + err = os.WriteFile(dir+"/contexts.html", []byte( + `{{.Body}}`, + ), 0o600) + require.NoError(t, err) + + engine := New(dir, ".html") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err = engine.Render(&buf, "contexts", map[string]interface{}{ + "URL": "javascript:alert(1)", + "Attr": `" onmouseover="alert(1)`, + "Body": xssPayload, + "JS": ``, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, `href="#ZgotmplZ"`) + require.NotContains(t, result, `javascript:alert(1)`) + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, `onmouseover="alert(1)`) + require.NotContains(t, result, ``) +} diff --git a/jet/README.md b/jet/README.md index 123d592..d0ea925 100644 --- a/jet/README.md +++ b/jet/README.md @@ -88,7 +88,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Jet escapes HTML output by default and should be the default path for untrusted data. +- Layout composition should continue to use `embed()`, which receives already-rendered child output. +- Custom globals and helper functions are trusted code and should not emit raw HTML for untrusted input. +- Review any helper or library feature that returns trusted markup before using it with user-controlled data. diff --git a/jet/security_test.go b/jet/security_test.go new file mode 100644 index 0000000..fab9730 --- /dev/null +++ b/jet/security_test.go @@ -0,0 +1,79 @@ +package jet + +import ( + "bytes" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".jet") + engine.AddFunc("isAdmin", func(user string) bool { + return user == "admin" + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".jet") + engine.AddFunc("isAdmin", func(user string) bool { + return user == "admin" + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_HelperOutputIsEscaped(t *testing.T) { + t.Parallel() + dir, err := os.MkdirTemp(".", "") + require.NoError(t, err) + + defer func() { + err := os.RemoveAll(dir) + require.NoError(t, err) + }() + + err = os.WriteFile(dir+"/helper.jet", []byte(`{{ helper() }}
`), 0o600) + require.NoError(t, err) + + engine := New(dir, ".jet") + engine.AddFunc("helper", func() string { + return xssPayload + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err = engine.Render(&buf, "helper", nil) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} diff --git a/mustache/README.md b/mustache/README.md index 3aeea55..44c51e7 100644 --- a/mustache/README.md +++ b/mustache/README.md @@ -89,7 +89,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Mustache escapes HTML output by default when you use `{{value}}`. +- Triple-stash expressions such as `{{{value}}}` and ampersand tags disable escaping and should only be used for trusted, pre-sanitized markup. +- Layout composition should continue to use `{{{embed}}}`, but only with child content that has already been rendered. +- Treat any raw-output sections in templates as trust boundaries and keep untrusted data on the escaped path whenever possible. diff --git a/mustache/mustache.go b/mustache/mustache.go index d81adb7..429c2b4 100644 --- a/mustache/mustache.go +++ b/mustache/mustache.go @@ -6,6 +6,7 @@ import ( "log" "net/http" "os" + "path" "path/filepath" "strings" @@ -24,18 +25,51 @@ type Engine struct { } type fileSystemPartialProvider struct { + root string fileSystem http.FileSystem extension string } -func (p fileSystemPartialProvider) Get(path string) (string, error) { - buf, err := core.ReadFile(path+p.extension, p.fileSystem) +func (p fileSystemPartialProvider) Get(name string) (string, error) { + cleanName, err := sanitizePartialName(name) + if err != nil { + return "", err + } + + filename := cleanName + p.extension + if p.fileSystem == nil { + filename = filepath.Join(p.root, filepath.FromSlash(filename)) + } else if p.root != "" { + filename = path.Join(p.root, filename) + } + + buf, err := core.ReadFile(filename, p.fileSystem) return string(buf), err } +func sanitizePartialName(name string) (string, error) { + cleanName := strings.ReplaceAll(strings.TrimSpace(name), `\`, "/") + cleanName = path.Clean(cleanName) + + switch { + case cleanName == "." || cleanName == "..": + return "", fmt.Errorf("mustache: invalid partial path %q", name) + case path.IsAbs(cleanName): + return "", fmt.Errorf("mustache: invalid partial path %q", name) + case strings.HasPrefix(cleanName, "../"): + return "", fmt.Errorf("mustache: invalid partial path %q", name) + } + + return cleanName, nil +} + // New returns a Mustache render engine for Fiber func New(directory, extension string) *Engine { engine := &Engine{ + partialsProvider: &fileSystemPartialProvider{ + root: ".", + extension: extension, + }, Engine: core.Engine{ Directory: directory, Extension: extension, @@ -77,7 +111,7 @@ func (e *Engine) Load() error { e.Templates = make(map[string]*mustache.Template) // Loop trough each directory and register template files - walkFn := func(path string, info os.FileInfo, err error) error { + walkFn := func(filePath string, info os.FileInfo, err error) error { // Return error if exist if err != nil { return err @@ -89,13 +123,13 @@ func (e *Engine) Load() error { } // Skip file if it does not equal the given template extension - if len(e.Extension) >= len(path) || path[len(path)-len(e.Extension):] != e.Extension { + if len(e.Extension) >= len(filePath) || filePath[len(filePath)-len(e.Extension):] != e.Extension { return nil } // Get the relative file path // ./views/html/index.tmpl -> index.tmpl - rel, err := filepath.Rel(e.Directory, path) + rel, err := filepath.Rel(e.Directory, filePath) if err != nil { return err } @@ -108,7 +142,7 @@ func (e *Engine) Load() error { // name = strings.Replace(name, e.extension, "", -1) // Read the file // #gosec G304 - buf, err := core.ReadFile(path, e.FileSystem) + buf, err := core.ReadFile(filePath, e.FileSystem) if err != nil { return err } diff --git a/mustache/security_test.go b/mustache/security_test.go new file mode 100644 index 0000000..be4d61b --- /dev/null +++ b/mustache/security_test.go @@ -0,0 +1,87 @@ +package mustache + +import ( + "bytes" + "net/http" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".mustache") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", customMap{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".mustache") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", customMap{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_PartialProvider_AllowsSafePaths(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "partials"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "partials", "header.mustache"), []byte("Header"), 0o600)) + + provider := fileSystemPartialProvider{ + root: dir, + extension: ".mustache", + } + + content, err := provider.Get("partials/header") + require.NoError(t, err) + require.Equal(t, "Header", content) +} + +func Test_PartialProvider_RejectsPathTraversal(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "safe.mustache"), []byte("safe"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.mustache"), []byte("secret"), 0o600)) + + provider := fileSystemPartialProvider{ + fileSystem: http.Dir(dir), + extension: ".mustache", + } + + content, err := provider.Get("safe") + require.NoError(t, err) + require.Equal(t, "safe", content) + + _, err = provider.Get("../secret") + require.Error(t, err) + require.ErrorContains(t, err, "invalid partial path") + + _, err = provider.Get("/secret") + require.Error(t, err) + require.ErrorContains(t, err, "invalid partial path") +} diff --git a/pug/README.md b/pug/README.md index d434090..cea4d45 100644 --- a/pug/README.md +++ b/pug/README.md @@ -85,7 +85,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Pug templates in this repository compile down to Go's `html/template`, so output benefits from contextual escaping by default. +- Layout composition should continue to use `{{embed}}`, which receives already-rendered child output. +- Custom helpers become a trust boundary when they return trusted `html/template` types such as `template.HTML`, `template.JS`, `template.URL`, or `template.CSS`. +- Do not convert user input into trusted `html/template` types unless you have already fully sanitized it for the target context. diff --git a/pug/security_test.go b/pug/security_test.go new file mode 100644 index 0000000..a7d0a3b --- /dev/null +++ b/pug/security_test.go @@ -0,0 +1,50 @@ +package pug + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".pug") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".pug") + engine.AddFunc("isAdmin", func(user string) bool { + return user == admin + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} diff --git a/slim/README.md b/slim/README.md index eb2e035..3dad33f 100644 --- a/slim/README.md +++ b/slim/README.md @@ -85,7 +85,14 @@ func main() { }, "layouts/main") }) - log.Fatal(app.Listen(":3000")) +log.Fatal(app.Listen(":3000")) } ``` + +## Security + +- Slim escapes output when you use `=` and writes raw output when you use `==`. +- Layout composition in this repository uses `== embed`, but only with child content that has already been rendered. +- Treat every `==` site as a trust boundary and keep untrusted data on the escaped `=` path whenever possible. +- Custom Slim functions are trusted code and should not emit raw markup for untrusted input. diff --git a/slim/security_test.go b/slim/security_test.go new file mode 100644 index 0000000..021f879 --- /dev/null +++ b/slim/security_test.go @@ -0,0 +1,74 @@ +package slim + +import ( + "bytes" + "os" + "testing" + + goslim "github.com/mattn/go-slim" + "github.com/stretchr/testify/require" +) + +const xssPayload = `` + +func Test_XSS(t *testing.T) { + t.Parallel() + engine := New("./views", ".slim") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "simple", map[string]interface{}{ + "Title": xssPayload, + }) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} + +func Test_Layout_DoesNotTrustUserProvidedEmbed(t *testing.T) { + t.Parallel() + engine := New("./views", ".slim") + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err := engine.Render(&buf, "index", map[string]interface{}{ + "Title": "Hello, World!", + "embed": xssPayload, + }, "layouts/main") + require.NoError(t, err) + + result := trim(buf.String()) + require.Contains(t, result, "Hello, World!") + require.NotContains(t, result, xssPayload) + require.NotContains(t, result, "<script>alert(1)</script>") +} + +func Test_HelperOutputIsEscaped(t *testing.T) { + t.Parallel() + dir, err := os.MkdirTemp(".", "") + require.NoError(t, err) + + defer func() { + err := os.RemoveAll(dir) + require.NoError(t, err) + }() + + err = os.WriteFile(dir+"/helper.slim", []byte(`p = helper()`), 0o600) + require.NoError(t, err) + + engine := New(dir, ".slim") + engine.AddFunc("helper", func(...goslim.Value) (goslim.Value, error) { + return xssPayload, nil + }) + require.NoError(t, engine.Load()) + + var buf bytes.Buffer + err = engine.Render(&buf, "helper", nil) + require.NoError(t, err) + + result := trim(buf.String()) + require.NotContains(t, result, xssPayload) + require.Contains(t, result, "<script>alert(1)</script>") +} diff --git a/template.go b/template.go index ae35619..b971e90 100644 --- a/template.go +++ b/template.go @@ -61,6 +61,9 @@ type Engine struct { // It is legal to overwrite elements of the default actions func (e *Engine) AddFunc(name string, fn interface{}) IEngineCore { e.Mutex.Lock() + if e.Funcmap == nil { + e.Funcmap = make(map[string]interface{}) + } e.Funcmap[name] = fn e.Mutex.Unlock() return e @@ -70,6 +73,9 @@ func (e *Engine) AddFunc(name string, fn interface{}) IEngineCore { // It is legal to overwrite elements of the default actions func (e *Engine) AddFuncMap(m map[string]interface{}) IEngineCore { e.Mutex.Lock() + if e.Funcmap == nil { + e.Funcmap = make(map[string]interface{}, len(m)) + } for name, fn := range m { e.Funcmap[name] = fn } diff --git a/template_test.go b/template_test.go new file mode 100644 index 0000000..194d0d5 --- /dev/null +++ b/template_test.go @@ -0,0 +1,50 @@ +package template_test + +import ( + "testing" + + template "github.com/gofiber/template/v2" +) + +func TestEngine_AddFunc_InitializesFuncMap(t *testing.T) { + t.Parallel() + + var engine template.Engine + engine.AddFunc("hello", func() string { + return "world" + }) + + if engine.Funcmap == nil { + t.Fatal("expected Funcmap to be initialized") + } + + if _, ok := engine.Funcmap["hello"]; !ok { + t.Fatal("expected hello func to be registered") + } +} + +func TestEngine_AddFuncMap_InitializesFuncMap(t *testing.T) { + t.Parallel() + + var engine template.Engine + engine.AddFuncMap(map[string]interface{}{ + "hello": func() string { + return "world" + }, + "goodbye": func() string { + return "moon" + }, + }) + + if len(engine.Funcmap) != 2 { + t.Fatalf("expected 2 funcs, got %d", len(engine.Funcmap)) + } + + if _, ok := engine.Funcmap["hello"]; !ok { + t.Fatal("expected hello func to be registered") + } + + if _, ok := engine.Funcmap["goodbye"]; !ok { + t.Fatal("expected goodbye func to be registered") + } +}