Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Django autoescape should be enable by default #326

Merged
merged 7 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions django/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ func main() {
// Create a new engine
engine := django.New("./views", ".django")

// Or from an embedded system
// See github.com/gofiber/embed for examples
// engine := html.NewFileSystem(http.Dir("./views", ".django"))
// Or from an embedded system
// See github.com/gofiber/embed for examples
// engine := html.NewFileSystem(http.Dir("./views", ".django"))

// Pass the engine to the Views
app := fiber.New(fiber.Config{
Expand Down Expand Up @@ -194,4 +194,44 @@ If you need to access a value in the template that doesn't adhere to the key nam
c.Render("index", fiber.Map{
"Fiber": "Hello, World!\n\nGreetings from Fiber Team",
"MyKey": c.Locals("my-key"),
})
})

### AutoEscape is enabled by default

When you create a new instance of the `Engine`, the auto-escape is **enabled by default**. This setting automatically escapes output, providing a critical security measure against Cross-Site Scripting (XSS) attacks.

### Disabling Auto-Escape

Auto-escaping can be disabled if necessary, using the `SetAutoEscape` method:

```go
engine := django.New("./views", ".django")
engine.SetAutoEscape(false)
```

### Setting AutoEscape using Django built-in template tags

- Explicitly turning off autoescaping for a section:
```django
{% autoescape off %}
{{ "<script>alert('Hello World');</script>" }}
{% endautoescape %}
```

- Turning autoescaping back on for a section:
```django
{% autoescape on %}
{{ "<script>alert('Hello World');</script>" }}
{% endautoescape %}
```
- It can also be done on a per variable basis using the *safe* built-in:
```django
<h1>{{ someSafeVar | safe }}</h1>
gaby marked this conversation as resolved.
Show resolved Hide resolved
{{ "<script>"|safe }}
```

### Security Implications of Disabling Auto-Escape

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.
24 changes: 22 additions & 2 deletions django/django.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ type Engine struct {
forwardPath bool
// templates
Templates map[string]*pongo2.Template
// set auto escape globally
AutoEscape bool
gaby marked this conversation as resolved.
Show resolved Hide resolved
}

// New returns a Django render engine for Fiber
Expand All @@ -36,6 +38,7 @@ func New(directory, extension string) *Engine {
LayoutName: "embed",
Funcmap: make(map[string]interface{}),
},
AutoEscape: true,
}
return engine
}
Expand All @@ -52,6 +55,7 @@ func NewFileSystem(fs http.FileSystem, extension string) *Engine {
LayoutName: "embed",
Funcmap: make(map[string]interface{}),
},
AutoEscape: true,
}
return engine
}
Expand All @@ -70,6 +74,7 @@ func NewPathForwardingFileSystem(fs http.FileSystem, directory, extension string
LayoutName: "embed",
Funcmap: make(map[string]interface{}),
},
AutoEscape: true,
forwardPath: true,
}
return engine
Expand Down Expand Up @@ -101,7 +106,8 @@ func (e *Engine) Load() error {
pongoset := pongo2.NewSet("default", pongoloader)
// Set template settings
pongoset.Globals.Update(e.Funcmap)
pongo2.SetAutoescape(false)
// Set autoescaping
pongo2.SetAutoescape(e.AutoEscape)

// Loop trough each Directory and register template files
walkFn := func(path string, info os.FileInfo, err error) error {
Expand Down Expand Up @@ -207,6 +213,11 @@ func isValidKey(key string) bool {
return true
}

// SetAutoEscape sets the auto-escape property of the template engine
func (e *Engine) SetAutoEscape(autoEscape bool) {
e.AutoEscape = autoEscape
}

// Render will render the template by name
func (e *Engine) Render(out io.Writer, name string, binding interface{}, layout ...string) error {
if !e.Loaded || e.ShouldReload {
Expand All @@ -231,7 +242,16 @@ func (e *Engine) Render(out io.Writer, name string, binding interface{}, layout
if bind == nil {
bind = make(map[string]interface{}, 1)
}
bind[e.LayoutName] = parsed

// Workaround for custom {{embed}} tag
// Mark the `embed` variable as safe
// it has already been escaped above
// e.LayoutName will be 'embed'
safeEmbed := pongo2.AsSafeValue(parsed)

// Add the safe value to the binding map
bind[e.LayoutName] = safeEmbed

lay := e.Templates[layout[0]]
if lay == nil {
return fmt.Errorf("LayoutName %s does not exist", layout[0])
Expand Down
31 changes: 31 additions & 0 deletions django/django_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,37 @@ func Test_Invalid_Layout(t *testing.T) {
require.Error(t, err)
}

func Test_XSS(t *testing.T) {
engine := New("./views", ".django")
require.NoError(t, engine.Load())

var buf bytes.Buffer
err := engine.Render(&buf, "index", map[string]interface{}{
"Title": "<script>alert('XSS')</script>",
}, "layouts/main")
require.NoError(t, err)

expect := `<!DOCTYPE html><html><head><title>Main</title></head><body><h2>Header</h2><h1>&lt;script&gt;alert(&#39;XSS&#39;)&lt;/script&gt;</h1><h2>Footer</h2></body></html>`
result := trim(buf.String())
require.Equal(t, expect, result)
}

func Test_XSS_WithAutoEscapeDisabled(t *testing.T) {
engine := New("./views", ".django")
engine.SetAutoEscape(false)
require.NoError(t, engine.Load())

var buf bytes.Buffer
err := engine.Render(&buf, "index", map[string]interface{}{
"Title": "<script>alert('XSS')</script>",
}, "layouts/main")
require.NoError(t, err)

expect := `<!DOCTYPE html><html><head><title>Main</title></head><body><h2>Header</h2><h1><script>alert('XSS')</script></h1><h2>Footer</h2></body></html>`
result := trim(buf.String())
require.Equal(t, expect, result)
}

func Benchmark_Django(b *testing.B) {
expectSimple := `<h1>Hello, World!</h1>`
expectExtended := `<!DOCTYPE html><html><head><title>Main</title></head><body><h2>Header</h2><h1>Hello, Admin!</h1><h2>Footer</h2></body></html>`
Expand Down
Loading