Skip to content

Commit

Permalink
Fix bugs found in fuzzing
Browse files Browse the repository at this point in the history
  • Loading branch information
yuin committed Jul 18, 2019
1 parent 36e42c4 commit 883918a
Show file tree
Hide file tree
Showing 19 changed files with 190 additions and 83 deletions.
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@

# Output of the go coverage tool, specifically when used with LiteIDE
*.out

.DS_Store
fuzz/corpus
fuzz/crashers
fuzz/suppressions
fuzz/fuzz-fuzz.zip
11 changes: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
.PHONY: test
.PHONY: test fuzz

test:
go test -coverprofile=profile.out -coverpkg=github.com/yuin/goldmark,github.com/yuin/goldmark/ast,github.com/yuin/goldmark/extension,github.com/yuin/goldmark/extension/ast,github.com/yuin/goldmark/parser,github.com/yuin/goldmark/renderer,github.com/yuin/goldmark/renderer/html,github.com/yuin/goldmark/text,github.com/yuin/goldmark/util ./...

cov: test
go tool cover -html=profile.out

fuzz:
which go-fuzz 2>&1 > /dev/null || (GO111MODULE=off go get -u github.com/dvyukov/go-fuzz/go-fuzz github.com/dvyukov/go-fuzz/go-fuzz-build; GO111MODULE=off go get -d github.com/dvyukov/go-fuzz-corpus; true)
cd ./fuzz && go-fuzz-build
rm -rf ./fuzz/corpus
rm -rf ./fuzz/crashers
rm -rf ./fuzz/suppressions
rm -f ./fuzz/fuzz-fuzz.zip
cd ./fuzz && go-fuzz
7 changes: 2 additions & 5 deletions ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ func (n *BaseNode) SetAttribute(name, value []byte) {
}
}
n.attributes = append(n.attributes, Attribute{name, value})
return
}

// Attribute implements Node.Attribute.
Expand Down Expand Up @@ -396,10 +395,8 @@ func DumpHelper(v Node, source []byte, level int, kv map[string]string, cb func(
fmt.Printf("\"\n")
fmt.Printf("%sHasBlankPreviousLines: %v\n", indent2, v.HasBlankPreviousLines())
}
if kv != nil {
for name, value := range kv {
fmt.Printf("%s%s: %s\n", indent2, name, value)
}
for name, value := range kv {
fmt.Printf("%s%s: %s\n", indent2, name, value)
}
if cb != nil {
cb(level + 1)
Expand Down
21 changes: 12 additions & 9 deletions extension/definition_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (b *definitionListParser) Open(parent gast.Node, reader text.Reader, pc par
}
line, _ := reader.PeekLine()
pos := pc.BlockOffset()
if line[pos] != ':' {
if pos < 0 || line[pos] != ':' {
return nil, parser.NoChildren
}

Expand Down Expand Up @@ -105,10 +105,13 @@ func NewDefinitionDescriptionParser() parser.BlockParser {
func (b *definitionDescriptionParser) Open(parent gast.Node, reader text.Reader, pc parser.Context) (gast.Node, parser.State) {
line, _ := reader.PeekLine()
pos := pc.BlockOffset()
if line[pos] != ':' {
if pos < 0 || line[pos] != ':' {
return nil, parser.NoChildren
}
list, _ := parent.(*ast.DefinitionList)
if list == nil {
return nil, parser.NoChildren
}
para := list.TemporaryParagraph
list.TemporaryParagraph = nil
if para != nil {
Expand Down Expand Up @@ -183,18 +186,18 @@ func (r *DefinitionListHTMLRenderer) RegisterFuncs(reg renderer.NodeRendererFunc

func (r *DefinitionListHTMLRenderer) renderDefinitionList(w util.BufWriter, source []byte, n gast.Node, entering bool) (gast.WalkStatus, error) {
if entering {
w.WriteString("<dl>\n")
_, _ = w.WriteString("<dl>\n")
} else {
w.WriteString("</dl>\n")
_, _ = w.WriteString("</dl>\n")
}
return gast.WalkContinue, nil
}

func (r *DefinitionListHTMLRenderer) renderDefinitionTerm(w util.BufWriter, source []byte, n gast.Node, entering bool) (gast.WalkStatus, error) {
if entering {
w.WriteString("<dt>")
_, _ = w.WriteString("<dt>")
} else {
w.WriteString("</dt>\n")
_, _ = w.WriteString("</dt>\n")
}
return gast.WalkContinue, nil
}
Expand All @@ -203,12 +206,12 @@ func (r *DefinitionListHTMLRenderer) renderDefinitionDescription(w util.BufWrite
if entering {
n := node.(*ast.DefinitionDescription)
if n.IsTight {
w.WriteString("<dd>")
_, _ = w.WriteString("<dd>")
} else {
w.WriteString("<dd>\n")
_, _ = w.WriteString("<dd>\n")
}
} else {
w.WriteString("</dd>\n")
_, _ = w.WriteString("</dd>\n")
}
return gast.WalkContinue, nil
}
Expand Down
55 changes: 30 additions & 25 deletions extension/footnote.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ func NewFootnoteBlockParser() parser.BlockParser {
func (b *footnoteBlockParser) Open(parent gast.Node, reader text.Reader, pc parser.Context) (gast.Node, parser.State) {
line, segment := reader.PeekLine()
pos := pc.BlockOffset()
if line[pos] != '[' {
if pos < 0 || line[pos] != '[' {
return nil, parser.NoChildren
}
pos++
if pos > len(line)-1 || line[pos] != '^' {
return nil, parser.NoChildren
}
open := pos + 1
closes := -1
closes := 0
closure := util.FindClosure(line[pos+1:], '[', ']', false, false)
if closure > -1 {
closes = pos + 1 + closure
Expand All @@ -52,10 +52,15 @@ func (b *footnoteBlockParser) Open(parent gast.Node, reader text.Reader, pc pars
if util.IsBlank(label) {
return nil, parser.NoChildren
}
item := ast.NewFootnote(label)

pos = pos + 2 + closes - open + 2
if pos >= len(line) {
reader.Advance(pos)
return item, parser.NoChildren
}
childpos, padding := util.IndentPosition(line[pos:], pos, 1)
reader.AdvanceAndSetPadding(pos+childpos, padding)
item := ast.NewFootnote(label)
return item, parser.HasChildren
}

Expand Down Expand Up @@ -207,13 +212,13 @@ func (r *FootnoteHTMLRenderer) renderFootnoteLink(w util.BufWriter, source []byt
if entering {
n := node.(*ast.FootnoteLink)
is := strconv.Itoa(n.Index)
w.WriteString(`<sup id="fnref:`)
w.WriteString(is)
w.WriteString(`"><a href="#fn:`)
w.WriteString(is)
w.WriteString(`" class="footnote-ref" role="doc-noteref">`)
w.WriteString(is)
w.WriteString(`</a></sup>`)
_, _ = w.WriteString(`<sup id="fnref:`)
_, _ = w.WriteString(is)
_, _ = w.WriteString(`"><a href="#fn:`)
_, _ = w.WriteString(is)
_, _ = w.WriteString(`" class="footnote-ref" role="doc-noteref">`)
_, _ = w.WriteString(is)
_, _ = w.WriteString(`</a></sup>`)
}
return gast.WalkContinue, nil
}
Expand All @@ -222,12 +227,12 @@ func (r *FootnoteHTMLRenderer) renderFootnote(w util.BufWriter, source []byte, n
n := node.(*ast.Footnote)
is := strconv.Itoa(n.Index)
if entering {
w.WriteString(`<li id="fn:`)
w.WriteString(is)
w.WriteString(`" role="doc-endnote">`)
w.WriteString("\n")
_, _ = w.WriteString(`<li id="fn:`)
_, _ = w.WriteString(is)
_, _ = w.WriteString(`" role="doc-endnote">`)
_, _ = w.WriteString("\n")
} else {
w.WriteString("</li>\n")
_, _ = w.WriteString("</li>\n")
}
return gast.WalkContinue, nil
}
Expand All @@ -238,20 +243,20 @@ func (r *FootnoteHTMLRenderer) renderFootnoteList(w util.BufWriter, source []byt
tag = "div"
}
if entering {
w.WriteString("<")
w.WriteString(tag)
w.WriteString(` class="footnotes" role="doc-endnotes">`)
_, _ = w.WriteString("<")
_, _ = w.WriteString(tag)
_, _ = w.WriteString(` class="footnotes" role="doc-endnotes">`)
if r.Config.XHTML {
w.WriteString("\n<hr />\n")
_, _ = w.WriteString("\n<hr />\n")
} else {
w.WriteString("\n<hr>\n")
_, _ = w.WriteString("\n<hr>\n")
}
w.WriteString("<ol>\n")
_, _ = w.WriteString("<ol>\n")
} else {
w.WriteString("</ol>\n")
w.WriteString("<")
w.WriteString(tag)
w.WriteString(">\n")
_, _ = w.WriteString("</ol>\n")
_, _ = w.WriteString("<")
_, _ = w.WriteString(tag)
_, _ = w.WriteString(">\n")
}
return gast.WalkContinue, nil
}
Expand Down
20 changes: 10 additions & 10 deletions extension/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,34 +169,34 @@ func (r *TableHTMLRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegistere

func (r *TableHTMLRenderer) renderTable(w util.BufWriter, source []byte, n gast.Node, entering bool) (gast.WalkStatus, error) {
if entering {
w.WriteString("<table>\n")
_, _ = w.WriteString("<table>\n")
} else {
w.WriteString("</table>\n")
_, _ = w.WriteString("</table>\n")
}
return gast.WalkContinue, nil
}

func (r *TableHTMLRenderer) renderTableHeader(w util.BufWriter, source []byte, n gast.Node, entering bool) (gast.WalkStatus, error) {
if entering {
w.WriteString("<thead>\n")
w.WriteString("<tr>\n")
_, _ = w.WriteString("<thead>\n")
_, _ = w.WriteString("<tr>\n")
} else {
w.WriteString("</tr>\n")
w.WriteString("</thead>\n")
_, _ = w.WriteString("</tr>\n")
_, _ = w.WriteString("</thead>\n")
if n.NextSibling() != nil {
w.WriteString("<tbody>\n")
_, _ = w.WriteString("<tbody>\n")
}
}
return gast.WalkContinue, nil
}

func (r *TableHTMLRenderer) renderTableRow(w util.BufWriter, source []byte, n gast.Node, entering bool) (gast.WalkStatus, error) {
if entering {
w.WriteString("<tr>\n")
_, _ = w.WriteString("<tr>\n")
} else {
w.WriteString("</tr>\n")
_, _ = w.WriteString("</tr>\n")
if n.Parent().LastChild() == n {
w.WriteString("</tbody>\n")
_, _ = w.WriteString("</tbody>\n")
}
}
return gast.WalkContinue, nil
Expand Down
28 changes: 28 additions & 0 deletions fuzz/fuzz.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package fuzz

import (
"bytes"
"github.com/yuin/goldmark"
"github.com/yuin/goldmark/extension"
"github.com/yuin/goldmark/renderer/html"
)

func Fuzz(data []byte) int {
markdown := goldmark.New(
goldmark.WithRendererOptions(
html.WithUnsafe(),
),
goldmark.WithExtensions(
extension.DefinitionList,
extension.Footnote,
extension.GFM,
extension.Typographer,
),
)
var b bytes.Buffer
if err := markdown.Convert(data, &b); err != nil {
return 0
}

return 1
}
41 changes: 41 additions & 0 deletions fuzz/fuzz_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package fuzz

import (
"bytes"
"fmt"
"io/ioutil"
"testing"

"github.com/yuin/goldmark"
"github.com/yuin/goldmark/extension"
"github.com/yuin/goldmark/renderer/html"
"github.com/yuin/goldmark/util"
)

var _ = fmt.Printf

func TestFuzz(t *testing.T) {
crasher := "6dff3d03167cb144d4e2891edac76ee740a77bc7"
data, err := ioutil.ReadFile("crashers/" + crasher)
if err != nil {
return
}
fmt.Printf("%s\n", util.VisualizeSpaces(data))
fmt.Println("||||||||||||||||||||||")
markdown := goldmark.New(
goldmark.WithRendererOptions(
html.WithUnsafe(),
),
goldmark.WithExtensions(
extension.DefinitionList,
extension.Footnote,
extension.GFM,
extension.Typographer,
),
)
var b bytes.Buffer
if err := markdown.Convert(data, &b); err != nil {
panic(err)
}
fmt.Println(b.String())
}
3 changes: 3 additions & 0 deletions parser/atx_heading.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ func NewATXHeadingParser(opts ...HeadingOption) BlockParser {
func (b *atxHeadingParser) Open(parent ast.Node, reader text.Reader, pc Context) (ast.Node, State) {
line, segment := reader.PeekLine()
pos := pc.BlockOffset()
if pos < 0 {
return nil, NoChildren
}
i := pos
for ; i < len(line) && line[i] == '#'; i++ {
}
Expand Down
2 changes: 1 addition & 1 deletion parser/code_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (b *codeBlockParser) Close(node ast.Node, reader text.Reader, pc Context) {
lines := node.Lines()
length := lines.Len() - 1
source := reader.Source()
for {
for length >= 0 {
line := lines.At(length)
if util.IsBlank(line.Value(source)) {
length--
Expand Down
2 changes: 1 addition & 1 deletion parser/fcode_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var fencedCodeBlockInfoKey = NewContextKey()
func (b *fencedCodeBlockParser) Open(parent ast.Node, reader text.Reader, pc Context) (ast.Node, State) {
line, segment := reader.PeekLine()
pos := pc.BlockOffset()
if line[pos] != '`' && line[pos] != '~' {
if pos < 0 || (line[pos] != '`' && line[pos] != '~') {
return nil, NoChildren
}
findent := pos
Expand Down
2 changes: 1 addition & 1 deletion parser/html_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (b *htmlBlockParser) Open(parent ast.Node, reader text.Reader, pc Context)
var node *ast.HTMLBlock
line, segment := reader.PeekLine()
last := pc.LastOpenedBlock().Node
if pos := pc.BlockOffset(); line[pos] != '<' {
if pos := pc.BlockOffset(); pos < 0 || line[pos] != '<' {
return nil, NoChildren
}

Expand Down
Loading

0 comments on commit 883918a

Please sign in to comment.