From fa0f304acdb51af104878be6368d08c51fd9c038 Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Thu, 18 Mar 2021 14:18:16 -0500 Subject: [PATCH 1/7] Minor cleanups, plus " " is not a reasonable default directory for FileProvider --- mustache.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mustache.go b/mustache.go index d304fdb..a4c99ae 100644 --- a/mustache.go +++ b/mustache.go @@ -92,7 +92,7 @@ type partialElement struct { prov PartialProvider } -// Template represents a compilde mustache template +// Template represents a compiled mustache template type Template struct { data string otag string @@ -231,7 +231,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{ @@ -528,7 +528,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 { @@ -571,7 +571,7 @@ func renderSection(section *sectionElement, contextChain []interface{}, buf io.W 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 { @@ -717,7 +717,7 @@ func ParseString(data string) (*Template, error) { func ParseStringRaw(data string, forceRaw bool) (*Template, error) { cwd := os.Getenv("CWD") partials := &FileProvider{ - Paths: []string{cwd, " "}, + Paths: []string{cwd}, } return ParseStringPartialsRaw(data, partials, forceRaw) @@ -752,7 +752,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) From 122b1e6d6b36061e080fa5052c6ec0e11f4e5201 Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Thu, 18 Mar 2021 14:19:40 -0500 Subject: [PATCH 2/7] Only open partial file once, filter unsafe partial names by default --- partials.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/partials.go b/partials.go index f6ea308..4189a89 100644 --- a/partials.go +++ b/partials.go @@ -1,10 +1,12 @@ package mustache import ( + "fmt" "io/ioutil" "os" "path" "regexp" + "strings" ) // PartialProvider comprises the behaviors required of a struct to be able to provide partials to the mustache rendering @@ -19,15 +21,26 @@ type PartialProvider interface { // FileProvider implements the PartialProvider interface by providing partials drawn from a filesystem. When a partial // named `NAME` is requested, FileProvider searches each listed path for a file named as `NAME` followed by any of the // listed extensions. The default for `Paths` is to search the current working directory. The default for `Extensions` -// is to examine, in order, no extension; then ".mustache"; then ".stache". +// is to examine, in order, no extension; then ".mustache"; then ".stache". If Unsafe is set, partial names are allowed +// to begin with '.' or '..' after cleaning, meaning they can potentially refer to files outside any of the listed +// directory paths. type FileProvider struct { Paths []string Extensions []string + Unsafe bool } // Get accepts the name of a partial and returns the parsed partial. func (fp *FileProvider) Get(name string) (string, error) { - var filename string + var cleanname string + if fp.Unsafe { + cleanname = name + } else { + cleanname = path.Clean(name) + if strings.HasPrefix(cleanname, ".") { + return "", fmt.Errorf("unsafe partial name passed to FileProvider: %s", name) + } + } var paths []string if fp.Paths != nil { @@ -43,23 +56,24 @@ func (fp *FileProvider) Get(name string) (string, error) { exts = []string{"", ".mustache", ".stache"} } + var f *os.File + var err error for _, p := range paths { for _, e := range exts { - name := path.Join(p, name+e) - f, err := os.Open(name) + pname := path.Join(p, name+e) + f, err = os.Open(pname) if err == nil { - filename = name - f.Close() break } } } - if filename == "" { + if f == nil { return "", nil } + defer f.Close() - data, err := ioutil.ReadFile(filename) + data, err := ioutil.ReadAll(f) if err != nil { return "", err } From 31606a967d50d1947e8bc0a1e79d136fd2968c7d Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Thu, 18 Mar 2021 14:20:11 -0500 Subject: [PATCH 3/7] Remove redundant type declarations --- spec_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/spec_test.go b/spec_test.go index 8ba495d..84a279a 100644 --- a/spec_test.go +++ b/spec_test.go @@ -10,7 +10,7 @@ import ( ) var enabledTests = map[string]map[string]bool{ - "comments.json": map[string]bool{ + "comments.json": { "Inline": true, "Multiline": true, "Standalone": true, @@ -22,8 +22,8 @@ var enabledTests = map[string]map[string]bool{ "Indented Multiline Standalone": true, "Indented Inline": true, "Surrounding Whitespace": true, - }, - "delimiters.json": map[string]bool{ + }, + "delimiters.json": { "Pair Behavior": true, "Special Characters": true, "Sections": true, @@ -38,8 +38,8 @@ var enabledTests = map[string]map[string]bool{ "Standalone Line Endings": true, "Standalone Without Previous Line": true, "Standalone Without Newline": true, - }, - "interpolation.json": map[string]bool{ + }, + "interpolation.json": { "No Interpolation": true, "Basic Interpolation": true, // disabled b/c Go uses """ in place of """ @@ -72,8 +72,8 @@ var enabledTests = map[string]map[string]bool{ "Interpolation With Padding": true, "Triple Mustache With Padding": true, "Ampersand With Padding": true, - }, - "inverted.json": map[string]bool{ + }, + "inverted.json": { "Falsey": true, "Truthy": true, "Context": true, @@ -95,8 +95,8 @@ var enabledTests = map[string]map[string]bool{ "Standalone Line Endings": true, "Standalone Without Previous Line": true, "Standalone Without Newline": true, - }, - "partials.json": map[string]bool{ + }, + "partials.json": { "Basic Behavior": true, "Failed Lookup": true, "Context": true, @@ -108,8 +108,8 @@ var enabledTests = map[string]map[string]bool{ "Standalone Without Newline": true, "Standalone Indentation": true, "Padding Whitespace": true, - }, - "sections.json": map[string]bool{ + }, + "sections.json": { "Truthy": true, "Falsey": true, "Context": true, @@ -136,7 +136,7 @@ var enabledTests = map[string]map[string]bool{ "Standalone Without Previous Line": true, "Standalone Without Newline": true, "Padding": true, - }, + }, "~lambdas.json": nil, // not implemented } From a69d40118e644559daeff0db8afb8c5c4c15990a Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Thu, 18 Mar 2021 14:20:50 -0500 Subject: [PATCH 4/7] Fix error message comparison, add test for partial safety enforcement --- mustache_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/mustache_test.go b/mustache_test.go index 836dbe4..d4c36e0 100644 --- a/mustache_test.go +++ b/mustache_test.go @@ -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()) } } @@ -300,6 +300,20 @@ 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 TestSectionPartial(t *testing.T) { filename := path.Join(path.Join(os.Getenv("PWD"), "tests"), "test3.mustache") From 3c40c75cd6994db124b3ce425804b329ac3b2504 Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Thu, 18 Mar 2021 14:37:25 -0500 Subject: [PATCH 5/7] Fix getcwd --- mustache.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mustache.go b/mustache.go index a4c99ae..e11cb3c 100644 --- a/mustache.go +++ b/mustache.go @@ -715,7 +715,10 @@ 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}, } From 9ae98b3dd61cee8ca73e619fff001fdad8cf5760 Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Mon, 22 Mar 2021 13:07:18 -0500 Subject: [PATCH 6/7] Add fuzzing, fix crash found by fuzzing --- mustache.go | 4 ++-- mustache_fuzz.go | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 mustache_fuzz.go diff --git a/mustache.go b/mustache.go index e11cb3c..08be5ae 100644 --- a/mustache.go +++ b/mustache.go @@ -370,8 +370,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) diff --git a/mustache_fuzz.go b/mustache_fuzz.go new file mode 100644 index 0000000..8b2c0ea --- /dev/null +++ b/mustache_fuzz.go @@ -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 +} From c5ff65f2ff676cfd5dcc652ba39f2474e631a0d3 Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Wed, 24 Mar 2021 16:41:52 -0500 Subject: [PATCH 7/7] Add support for JSON and plain text modes --- mustache.go | 69 +++++++++++++++++++++++---- mustache_test.go | 122 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 9 deletions(-) diff --git a/mustache.go b/mustache.go index 08be5ae..16d6507 100644 --- a/mustache.go +++ b/mustache.go @@ -11,6 +11,7 @@ import ( "reflect" "strconv" "strings" + "unicode" ) var ( @@ -92,7 +93,19 @@ type partialElement struct { prov PartialProvider } -// Template represents a compiled 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 @@ -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 { @@ -565,7 +581,7 @@ 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 @@ -602,7 +618,7 @@ 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 } } @@ -610,7 +626,33 @@ func renderSection(section *sectionElement, contextChain []interface{}, buf io.W 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) @@ -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: @@ -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 } } @@ -739,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 { @@ -779,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 { diff --git a/mustache_test.go b/mustache_test.go index d4c36e0..2a5a8b8 100644 --- a/mustache_test.go +++ b/mustache_test.go @@ -314,6 +314,128 @@ func TestPartialSafety(t *testing.T) { } } +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": ``, "b": "}o&o{", "c": "\t"}, + Result: " }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")