Cyphrme / Coze

Coze is a cryptographic JSON messaging specification.
https://cyphr.me/coze
BSD 3-Clause "New" or "Revised" License
110 stars 3 forks source link

JSON round-trip fails for MapSlice #11

Closed clfs closed 1 year ago

clfs commented 1 year ago

It's possible to successfully unmarshal valid JSON into a MapSlice, but then fail to marshal that same MapSlice back into JSON.

I'm assuming this isn't desired behavior, but I could be wrong.

POC: https://go.dev/play/p/MlNV74p2cCY

package main

import (
    "encoding/json"
    "log"

    "github.com/cyphrme/coze"
)

func main() {
    b := []byte("{\"\x7f\":{}}") // discovered via fuzzing
    if !json.Valid(b) {
        log.Fatal("invalid JSON")
    }

    var ms coze.MapSlice
    if err := json.Unmarshal(b, &ms); err != nil {
        log.Fatalf("json.Unmarshal: %v", err)
    }

    if _, err := json.Marshal(ms); err != nil {
        log.Fatalf("json.Marshal: %v", err) // fails here
    }
}
zamicol commented 1 year ago

We're taking a look.

zamicol commented 1 year ago

Fixed by b02966e. The problem was MapSlice.MarshalJSON was doing fmt with a %+q when it should just call Marshal again.

horvski commented 1 year ago

@clfs Would you mind sharing the fuzzing code you used to discover this?

clfs commented 1 year ago

Sure! Drop this in mapslice_test.go:

func FuzzMapSliceJSONRoundTrip(f *testing.F) {
    f.Add([]byte(`{"foo": [123, 456]}`))
    f.Fuzz(func(t *testing.T, b []byte) {
        var m MapSlice
        if err := json.Unmarshal(b, &m); err != nil {
            return
        }

        data, err := json.Marshal(m)
        if err != nil {
            t.Fatalf("marshal: %v", err)
        }

        var m2 MapSlice
        if err := json.Unmarshal(data, &m2); err != nil {
            t.Fatalf("unmarshal again: %v", err)
        }

        data2, err := json.Marshal(m2)
        if err != nil {
            t.Fatalf("marshal again: %v", err)
        }

        if !bytes.Equal(data, data2) {
            t.Fatalf("marshals differ: %q != %q", data, data2)
        }
    })
}

Then run it (I deleted the cache, so the failing input looks different):

calvin@mbp Coze % go test -fuzz FuzzMapSliceJSONRoundTrip .
fuzz: elapsed: 0s, gathering baseline coverage: 0/1 completed
fuzz: elapsed: 0s, gathering baseline coverage: 1/1 completed, now fuzzing with 10 workers
fuzz: elapsed: 3s, execs: 763910 (254609/sec), new interesting: 228 (total: 229)
fuzz: elapsed: 6s, execs: 1601985 (279341/sec), new interesting: 272 (total: 273)
fuzz: elapsed: 9s, execs: 2444368 (280836/sec), new interesting: 291 (total: 292)
fuzz: elapsed: 12s, execs: 3289903 (281844/sec), new interesting: 306 (total: 307)
fuzz: elapsed: 15s, execs: 4126384 (278823/sec), new interesting: 315 (total: 316)
fuzz: minimizing 88-byte failing input file
fuzz: elapsed: 18s, minimizing
fuzz: elapsed: 18s, minimizing
--- FAIL: FuzzMapSliceJSONRoundTrip (18.23s)
    --- FAIL: FuzzMapSliceJSONRoundTrip (0.00s)
        fuzz_test.go:68: marshal: json: error calling MarshalJSON for type coze.MapSlice: invalid character 'x' in string escape code

    Failing input written to testdata/fuzz/FuzzMapSliceJSONRoundTrip/db149316aeed8ab9
    To re-run:
    go test -run=FuzzMapSliceJSONRoundTrip/db149316aeed8ab9
FAIL
exit status 1
FAIL    github.com/cyphrme/coze 18.524s
calvin@mbp Coze % cat testdata/fuzz/FuzzMapSliceJSONRoundTrip/db149316aeed8ab9
go test fuzz v1
[]byte("{\"\x7f\":0}")
calvin@mbp Coze %