Skip to content

Commit

Permalink
fuzz:
Browse files Browse the repository at this point in the history
- for parse/write/parse check, change second parse to parse the write
  of the parsed value instead of the original input

encode:
- invalidKeyRune: treat utf8.RuneError as invalid rune
- add invalidKey, invalidKeyString funcs
  - checks len(key) == 0, invalidKeyRune, and utf8.Valid
- needsQuotedValueRune: if value contains utf8.RuneError quote the
  value. a literal k=\ufffd encodes to k=\"\\ufffd\"
- writeStringValue, writeBytesValue: quote any invalid utf8 string
- the above two changes fix the "reserialized data does not match"
  error found during fuzz testing

decode:
- reject invalidKey as parse error

jsonstring:
- remove "&& size == 1" when checking for rune decode error

fuzz testing output:
- .quoted:
  "0=\"\xbd\x00\""
- .output:
  panic: reserialized data does not match:
  "0=\"\\ufffd\\u0000\"\n"
  "0=\"�\\u0000\"\n"
  • Loading branch information
judwhite committed Nov 14, 2016
1 parent 282c134 commit 52e29b1
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 10 deletions.
14 changes: 14 additions & 0 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,19 @@ func (dec *Decoder) ScanKeyval() bool {
return false

key:
const invalidKeyError = "invalid key"

start := dec.pos
for p, c := range line[dec.pos:] {
switch {
case c == '=':
dec.pos += p
if dec.pos > start {
dec.key = line[start:dec.pos]
if invalidKey(dec.key) {
dec.syntaxError(invalidKeyError)
return false
}
}
if dec.key == nil {
dec.unexpectedByte(c)
Expand All @@ -89,13 +95,21 @@ key:
dec.pos += p
if dec.pos > start {
dec.key = line[start:dec.pos]
if invalidKey(dec.key) {
dec.syntaxError(invalidKeyError)
return false
}
}
return true
}
}
dec.pos = len(line)
if dec.pos > start {
dec.key = line[start:dec.pos]
if invalidKey(dec.key) {
dec.syntaxError(invalidKeyError)
return false
}
}
return true

Expand Down
21 changes: 15 additions & 6 deletions encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"reflect"
"strings"
"unicode/utf8"
)

// MarshalKeyvals returns the logfmt encoding of keyvals, a variadic sequence
Expand Down Expand Up @@ -165,19 +166,27 @@ func writeKey(w io.Writer, key interface{}) error {
}

func invalidKeyRune(r rune) bool {
return r <= ' ' || r == '=' || r == '"'
return r <= ' ' || r == '=' || r == '"' || r == utf8.RuneError
}

func invalidKeyString(key string) bool {
return len(key) == 0 || strings.IndexFunc(key, invalidKeyRune) != -1 || !utf8.ValidString(key)
}

func invalidKey(key []byte) bool {
return len(key) == 0 || bytes.IndexFunc(key, invalidKeyRune) != -1 || !utf8.Valid(key)
}

func writeStringKey(w io.Writer, key string) error {
if len(key) == 0 || strings.IndexFunc(key, invalidKeyRune) != -1 {
if invalidKeyString(key) {
return ErrInvalidKey
}
_, err := io.WriteString(w, key)
return err
}

func writeBytesKey(w io.Writer, key []byte) error {
if len(key) == 0 || bytes.IndexFunc(key, invalidKeyRune) != -1 {
if invalidKey(key) {
return ErrInvalidKey
}
_, err := w.Write(key)
Expand Down Expand Up @@ -223,14 +232,14 @@ func writeValue(w io.Writer, value interface{}) error {
}

func needsQuotedValueRune(r rune) bool {
return r <= ' ' || r == '=' || r == '"'
return r <= ' ' || r == '=' || r == '"' || r == utf8.RuneError
}

func writeStringValue(w io.Writer, value string, ok bool) error {
var err error
if ok && value == "null" {
_, err = io.WriteString(w, `"null"`)
} else if strings.IndexFunc(value, needsQuotedValueRune) != -1 {
} else if strings.IndexFunc(value, needsQuotedValueRune) != -1 || !utf8.ValidString(value) {
_, err = writeQuotedString(w, value)
} else {
_, err = io.WriteString(w, value)
Expand All @@ -240,7 +249,7 @@ func writeStringValue(w io.Writer, value string, ok bool) error {

func writeBytesValue(w io.Writer, value []byte) error {
var err error
if bytes.IndexFunc(value, needsQuotedValueRune) >= 0 {
if bytes.IndexFunc(value, needsQuotedValueRune) >= 0 || !utf8.Valid(value) {
_, err = writeQuotedBytes(w, value)
} else {
_, err = w.Write(value)
Expand Down
2 changes: 1 addition & 1 deletion fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func Fuzz(data []byte) int {
if err = write(parsed, &w1); err != nil {
panic(err)
}
parsed, err = parse(data)
parsed, err = parse(w1.Bytes())
if err != nil {
panic(err)
}
Expand Down
6 changes: 3 additions & 3 deletions jsonstring.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func writeQuotedString(w io.Writer, s string) (int, error) {
continue
}
c, size := utf8.DecodeRuneInString(s[i:])
if c == utf8.RuneError && size == 1 {
if c == utf8.RuneError {
if start < i {
buf.WriteString(s[start:i])
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func writeQuotedBytes(w io.Writer, s []byte) (int, error) {
continue
}
c, size := utf8.DecodeRune(s[i:])
if c == utf8.RuneError && size == 1 {
if c == utf8.RuneError {
if start < i {
buf.Write(s[start:i])
}
Expand Down Expand Up @@ -182,7 +182,7 @@ func unquoteBytes(s []byte) (t []byte, ok bool) {
continue
}
rr, size := utf8.DecodeRune(s[r:])
if rr == utf8.RuneError && size == 1 {
if rr == utf8.RuneError {
break
}
r += size
Expand Down

0 comments on commit 52e29b1

Please sign in to comment.