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 security hole in #50, plus minor code improvements #51

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
88 changes: 71 additions & 17 deletions mustache.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"reflect"
"strconv"
"strings"
"unicode"
)

var (
Expand Down Expand Up @@ -92,7 +93,19 @@ type partialElement struct {
prov PartialProvider
}

// Template represents a compilde mustache template
// EscapeMode indicates what sort of escaping to perform in template output.
// EscapeHTML is the default, and assumes the template is producing HTML.
// EscapeJSON switches to JSON escaping, for use cases such as generating Slack messages.
// Raw turns off escaping, for situations where you are absolutely sure you want plain text.
type EscapeMode int

const (
EscapeHTML EscapeMode = iota
EscapeJSON
Raw
)

// Template represents a compiled mustache template.
type Template struct {
data string
otag string
Expand All @@ -102,6 +115,9 @@ type Template struct {
elems []interface{}
forceRaw bool
partial PartialProvider
// OutputMode can be set to indicate what sort of output the template is expected to produce. This switches
// the escaping into an appropriate mode for HTML (default), JSON, or plain text.
OutputMode EscapeMode
}

type parseError struct {
Expand Down Expand Up @@ -231,7 +247,7 @@ func (tmpl *Template) readText() (*textReadingResult, error) {
}
}

mayStandalone := (i == 0 || tmpl.data[i-1] == '\n')
mayStandalone := i == 0 || tmpl.data[i-1] == '\n'

if mayStandalone {
return &textReadingResult{
Expand Down Expand Up @@ -370,8 +386,8 @@ func (tmpl *Template) parseSection(section *sectionElement) error {
}
section.elems = append(section.elems, partial)
case '=':
if tag[len(tag)-1] != '=' {
return parseError{tmpl.curline, "Invalid meta tag"}
if len(tag) < 2 || tag[len(tag)-1] != '=' {
return parseError{tmpl.curline, "invalid meta tag"}
}
tag = strings.TrimSpace(tag[1 : len(tag)-1])
newtags := strings.SplitN(tag, " ", 2)
Expand Down Expand Up @@ -528,7 +544,7 @@ Outer:
if allowMissing {
return reflect.Value{}, nil
}
return reflect.Value{}, fmt.Errorf("Missing variable %q", name)
return reflect.Value{}, fmt.Errorf("missing variable %q", name)
}

func isEmpty(v reflect.Value) bool {
Expand Down Expand Up @@ -565,13 +581,13 @@ loop:
return v
}

func renderSection(section *sectionElement, contextChain []interface{}, buf io.Writer) error {
func (tmpl *Template) renderSection(section *sectionElement, contextChain []interface{}, buf io.Writer) error {
value, err := lookup(contextChain, section.name, true)
if err != nil {
return err
}
var context = contextChain[len(contextChain)-1].(reflect.Value)
var contexts = []interface{}{}
var contexts []interface{}
// if the value is nil, check if it's an inverted section
isEmpty := isEmpty(value)
if isEmpty && !section.inverted || !isEmpty && section.inverted {
Expand Down Expand Up @@ -602,15 +618,41 @@ func renderSection(section *sectionElement, contextChain []interface{}, buf io.W
for _, ctx := range contexts {
chain2[0] = ctx
for _, elem := range section.elems {
if err := renderElement(elem, chain2, buf); err != nil {
if err := tmpl.renderElement(elem, chain2, buf); err != nil {
return err
}
}
}
return nil
}

func renderElement(element interface{}, contextChain []interface{}, buf io.Writer) error {
func JSONEscape(dest io.Writer, data string) {
for _, r := range data {
switch r {
case '"', '\\':
dest.Write([]byte("\\"))
dest.Write([]byte(string(r)))
case '\n':
dest.Write([]byte(`\n`))
case '\b':
dest.Write([]byte(`\b`))
case '\f':
dest.Write([]byte(`\f`))
case '\r':
dest.Write([]byte(`\r`))
case '\t':
dest.Write([]byte(`\t`))
default:
if unicode.IsControl(r) {
dest.Write([]byte(fmt.Sprintf("\\u%04x", r)))
} else {
dest.Write([]byte(string(r)))
}
}
}
}

func (tmpl *Template) renderElement(element interface{}, contextChain []interface{}, buf io.Writer) error {
switch elem := element.(type) {
case *textElement:
_, err := buf.Write(elem.text)
Expand All @@ -631,11 +673,20 @@ func renderElement(element interface{}, contextChain []interface{}, buf io.Write
fmt.Fprint(buf, val.Interface())
} else {
s := fmt.Sprint(val.Interface())
template.HTMLEscape(buf, []byte(s))
switch tmpl.OutputMode {
case EscapeJSON:
JSONEscape(buf, s)
case EscapeHTML:
template.HTMLEscape(buf, []byte(s))
case Raw:
if _, err = buf.Write([]byte(s)); err != nil {
return err
}
}
}
}
case *sectionElement:
if err := renderSection(elem, contextChain, buf); err != nil {
if err := tmpl.renderSection(elem, contextChain, buf); err != nil {
return err
}
case *partialElement:
Expand All @@ -652,7 +703,7 @@ func renderElement(element interface{}, contextChain []interface{}, buf io.Write

func (tmpl *Template) renderTemplate(contextChain []interface{}, buf io.Writer) error {
for _, elem := range tmpl.elems {
if err := renderElement(elem, contextChain, buf); err != nil {
if err := tmpl.renderElement(elem, contextChain, buf); err != nil {
return err
}
}
Expand Down Expand Up @@ -715,9 +766,12 @@ func ParseString(data string) (*Template, error) {
// be used to efficiently render the template multiple times with different data
// sources.
func ParseStringRaw(data string, forceRaw bool) (*Template, error) {
cwd := os.Getenv("CWD")
cwd, err := os.Getwd()
if err != nil {
return nil, err
}
partials := &FileProvider{
Paths: []string{cwd, " "},
Paths: []string{cwd},
}

return ParseStringPartialsRaw(data, partials, forceRaw)
Expand All @@ -736,7 +790,7 @@ func ParseStringPartials(data string, partials PartialProvider) (*Template, erro
// to efficiently render the template multiple times with different data
// sources.
func ParseStringPartialsRaw(data string, partials PartialProvider, forceRaw bool) (*Template, error) {
tmpl := Template{data, "{{", "}}", 0, 1, []interface{}{}, forceRaw, partials}
tmpl := Template{data, "{{", "}}", 0, 1, []interface{}{}, forceRaw, partials, EscapeHTML}
err := tmpl.parse()

if err != nil {
Expand All @@ -752,7 +806,7 @@ func ParseStringPartialsRaw(data string, partials PartialProvider, forceRaw bool
func ParseFile(filename string) (*Template, error) {
dirname, _ := path.Split(filename)
partials := &FileProvider{
Paths: []string{dirname, " "},
Paths: []string{dirname},
}

return ParseFilePartials(filename, partials)
Expand All @@ -776,7 +830,7 @@ func ParseFilePartialsRaw(filename string, forceRaw bool, partials PartialProvid
return nil, err
}

tmpl := Template{string(data), "{{", "}}", 0, 1, []interface{}{}, forceRaw, partials}
tmpl := Template{string(data), "{{", "}}", 0, 1, []interface{}{}, forceRaw, partials, EscapeHTML}
err = tmpl.parse()

if err != nil {
Expand Down
19 changes: 19 additions & 0 deletions mustache_fuzz.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// +build gofuzz

package mustache

// Fuzzing code for use with github.com/dvyukov/go-fuzz
//
// To use, in the main project directory do:
//
// go get -u github.com/dvyukov/go-fuzz/go-fuzz github.com/dvyukov/go-fuzz/go-fuzz-build
// go-fuzz-build
// go-fuzz

func Fuzz(data []byte) int {
_, err := ParseString(string(data))
if err == nil {
return 1
}
return 0
}
138 changes: 137 additions & 1 deletion mustache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestMissing(t *testing.T) {
output, err := Render(test.tmpl, test.context)
if err == nil {
t.Errorf("%q expected missing variable error but got %q", test.tmpl, output)
} else if !strings.Contains(err.Error(), "Missing variable") {
} else if !strings.Contains(err.Error(), "missing variable") {
t.Errorf("%q expected missing variable error but got %q", test.tmpl, err.Error())
}
}
Expand Down Expand Up @@ -300,6 +300,142 @@ func TestPartial(t *testing.T) {
compareTags(t, tmpl.Tags(), expectedTags)
}

func TestPartialSafety(t *testing.T) {
tmpl, err := ParseString("{{>../unsafe}}")
if err != nil {
t.Error(err)
}
txt, err := tmpl.Render(nil)
if err == nil {
t.Errorf("expected error for unsafe partial")
}
if txt != "" {
t.Errorf("expected unsafe partial to fail")
}
}

func TestJSONEscape(t *testing.T) {
tests := []struct{
Before string
After string
}{
{`'single quotes'`, `'single quotes'`},
{`"double quotes"`, `\"double quotes\"`},
{`\backslash\`, `\\backslash\\`},
{"some\tcontrol\ncharacters\x1c", `some\tcontrol\ncharacters\u001c`},
{ `🦜`, `🦜`},
}
var buf bytes.Buffer
for _, tst := range tests {
JSONEscape(&buf, tst.Before)
txt := buf.String()
if txt != tst.After {
t.Errorf("got %s expected %s", txt, tst.After)
}
buf.Reset()
}
}

func TestRenderRaw(t *testing.T) {
tests := []struct{
Template string
Data map[string]interface{}
Result string
}{
{
Template: `{{a}} {{b}} {{c}}`,
Data: map[string]interface{}{"a": `<a href="">`, "b": "}o&o{", "c": "\t"},
Result: "<a href=\"\"> }o&o{ \t",
},
}
for _, tst := range tests {
tmpl, err := ParseString(tst.Template)
if err != nil {
t.Error(err)
}
tmpl.OutputMode = Raw
txt, err := tmpl.Render(tst.Data)
if err != nil {
t.Error(err)
}
if txt != tst.Result {
t.Errorf("expected %s got %s", tst.Result, txt)
}
}
}

func TestRenderJSON(t *testing.T) {

type item struct {
Emoji string
Name string
}

tests := []struct{
Template string
Data map[string]interface{}
Result string
}{
{ Template: `{"a": "{{a}}", "b": "{{b}}", "c": "{{c}}"}`,
Data: map[string]interface{}{"a": "Text\nwith\tcontrols", "b": `"I said 'No!'"`, "c": "EOF\u001cHERE"},
Result: `{"a": "Text\nwith\tcontrols", "b": "\"I said 'No!'\"", "c": "EOF\u001cHERE"}` },
{
Template: `{"a": [""{{#a}},"{{.}}"{{/a}}]}`,
Data: map[string]interface{}{"a": []int{1,2,3}},
Result: `{"a": ["","1","2","3"]}`,
},
{
Template: `"{{#values}}{{Emoji}}{{Name}} {{/values}}"`,
Data: map[string]interface{}{
"values": interface{}([]item{
item{
Emoji: "🟡",
Name: "Rico",
},
item{
Emoji: "🟢",
Name: "Bruce",
},
item{
Emoji: "🔵",
Name: "Luna",
},
}),
},
Result: `"🟡Rico 🟢Bruce 🔵Luna "`,
},
}
for _, tst := range tests {
tmpl, err := ParseString(tst.Template)
if err != nil {
t.Error(err)
}
tmpl.OutputMode = EscapeJSON
txt, err := tmpl.Render(tst.Data)
if err != nil {
t.Error(err)
}
if txt != tst.Result {
t.Errorf("expected %s got %s", tst.Result, txt)
}
}
}

// Make sure bugs caught by fuzz testing don't creep back in
func TestCrashers(t *testing.T) {
crashers := []string{
`{{#}}{{#}}{{#}}{{#}}{{#}}{{=}}`,
`{{#}}{{#}}{{#}}{{#}}{{#}}{{#}}{{#}}{{#}}{{=}}`,
}
for i, c := range crashers {
t.Log(i)
_, err := ParseString(c)
if err == nil {
t.Error(err)
}
}
}

/*
func TestSectionPartial(t *testing.T) {
filename := path.Join(path.Join(os.Getenv("PWD"), "tests"), "test3.mustache")
Expand Down
Loading