Skip to content

Commit

Permalink
Merge pull request #50 from adamdecaf/validateOptionFLine-crash
Browse files Browse the repository at this point in the history
validators: fix crash from bounds check
  • Loading branch information
adamdecaf committed Jul 3, 2019
2 parents c004116 + 9a0d409 commit d7cc2ac
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{5010}TXID/000000000000000000000000000000 1 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
panic: runtime error: slice bounds out of range

goroutine 1 [running]:
github.com/moov-io/wire.(*validator).validateOptionFName(0xc00008a220, 0xc00009402e, 0x1, 0x0, 0x0)
/go/src/github.com/moov-io/wire/validators.go:575 +0x359
github.com/moov-io/wire.(*OriginatorOptionF).Validate(0xc00008a1c0, 0xc000094000, 0xb5)
/go/src/github.com/moov-io/wire/originatorOptionF.go:136 +0x12d
github.com/moov-io/wire.(*Reader).parseOriginatorOptionF(0xc00003fa70, 0x6, 0x60a801)
/go/src/github.com/moov-io/wire/reader.go:588 +0x250
github.com/moov-io/wire.(*Reader).parseLine(0xc00003fa70, 0xc000092000, 0xb5)
/go/src/github.com/moov-io/wire/reader.go:175 +0x10e9
github.com/moov-io/wire.(*Reader).Read(0xc00003fa70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
/go/src/github.com/moov-io/wire/reader.go:74 +0x138
github.com/moov-io/wire/test/fuzz-reader.Fuzz(0x7f1559424000, 0xb5, 0x200000, 0x4)
/go/src/github.com/moov-io/wire/test/fuzz-reader/reader.go:21 +0x193
go-fuzz-dep.Main(0xc00003ff80, 0x1, 0x1)
/tmp/go-fuzz-build187149637/goroot/src/go-fuzz-dep/main.go:36 +0x1b6
main.main()
/tmp/go-fuzz-build187149637/gopath/src/github.com/moov-io/wire/test/fuzz-reader/go.fuzz.main/main.go:15 +0x52
exit status 2
7 changes: 2 additions & 5 deletions validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,13 +563,10 @@ func (v *validator) validateOptionFLine(s string) error {
// Name Format: Must begin with Line Code 1 followed by a slash and at least one valid non-space character:
// e.g., 1/SMITH JOHN.
func (v *validator) validateOptionFName(s string) error {
if s == "" {
if utf8.RuneCountInString(s) < 3 {
return ErrOptionFName
}
switch s[:1] {
case
OptionFName:
default:
if s[:1] != OptionFName {
return ErrOptionFName
}
if s[1:2] != "/" {
Expand Down
28 changes: 28 additions & 0 deletions validators_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2019 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package wire

import (
"testing"
)

func TestValidators__validateOptionFName(t *testing.T) {
v := &validator{}
if err := v.validateOptionFName("1/SMITH JOHN"); err != nil {
t.Error(err)
}
if err := v.validateOptionFName("1/"); err == nil {
t.Error("expected error")
}
if err := v.validateOptionFName("1"); err == nil {
t.Error("expected error")
}
if err := v.validateOptionFName(""); err == nil {
t.Error("expected error")
}
if err := v.validateOptionFName(" /"); err == nil {
t.Error("expected error")
}
}
40 changes: 40 additions & 0 deletions wire_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2019 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package wire

import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
)

// TestWire__ReadCrashers will attempt to parse files which have previously been reported
// as crashing. These files are typically generated via fuzzing, but might also be reported
// by users.
func TestWire__ReadCrashers(t *testing.T) {
root := filepath.Join("test", "testdata", "crashers")
err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
if (err != nil && err != filepath.SkipDir) || info == nil || info.IsDir() {
return nil // Ignore SkipDir and directories
}
if strings.HasSuffix(path, ".output") {
return nil // go-fuzz makes these which contain the panic's trace
}

fd, err := os.Open(path)
if err != nil {
return fmt.Errorf("problem opening %s: %v", path, err)
}

// Read out test file and ensure we don't panic
NewReader(fd).Read()
return nil
})
if err != nil {
t.Fatal(err)
}
}

0 comments on commit d7cc2ac

Please sign in to comment.