test: manual ipld garbage fuzzing on messages v2

[rvagg]

This might just be a temporary branch but I wanted to throw some garbage, but valid, dag-cbor at the message decoder to see how easy it was to break; it turns out it's still pretty trivial to break but I don't really understand why. So this one's for @mvdan.

Any time it comes up with a top-level map it'll panic like this:

panic: interface conversion: datamodel.NodeAssembler is bindnode._errorAssembler, not *bindnode._assembler [recovered]
        panic: interface conversion: datamodel.NodeAssembler is bindnode._errorAssembler, not *bindnode._assembler

goroutine 19 [running]:
testing.tRunner.func1.2(0x8cdee0, 0xc00039eea0)
        /snap/go/9026/src/testing/testing.go:1143 +0x332
testing.tRunner.func1(0xc000102a80)
        /snap/go/9026/src/testing/testing.go:1146 +0x4b6
panic(0x8cdee0, 0xc00039eea0)
        /snap/go/9026/src/runtime/panic.go:965 +0x1b9
github.com/ipld/go-ipld-prime/node/bindnode.assemblerRepr(...)
        /home/rvagg/go/pkg/mod/github.com/ipld/go-ipld-prime@v0.14.5-0.20220214151842-d62ec3674f3a/node/bindnode/repr.go:541
github.com/ipld/go-ipld-prime/node/bindnode.(*_unionAssemblerRepr).AssembleValue(0xc0003becb0, 0xc00020dfd9, 0x3)
        /home/rvagg/go/pkg/mod/github.com/ipld/go-ipld-prime@v0.14.5-0.20220214151842-d62ec3674f3a/node/bindnode/repr.go:960 +0x27f
github.com/ipld/go-ipld-prime/node/bindnode.(*_unionAssemblerRepr).AssembleEntry(0xc0003becb0, 0xc00020dfd9, 0x3, 0x0, 0x0, 0x0, 0xc0003bec40)
        /home/rvagg/go/pkg/mod/github.com/ipld/go-ipld-prime@v0.14.5-0.20220214151842-d62ec3674f3a/node/bindnode/repr.go:971 +0x93
github.com/ipld/go-ipld-prime/codec/dagcbor.unmarshal2(0x7f12a0433270, 0xc0003a48c0, 0xaf69c0, 0xc0003ae230, 0xc0003bec40, 0xc000131d98, 0xdab701, 0x0, 0xc000131d48)
        /home/rvagg/go/pkg/mod/github.com/ipld/go-ipld-prime@v0.14.5-0.20220214151842-d62ec3674f3a/codec/dagcbor/unmarshal.go:133 +0xbbe
github.com/ipld/go-ipld-prime/codec/dagcbor.unmarshal1(0x7f12a0433270, 0xc0003a48c0, 0xaf69c0, 0xc0003ae230, 0xc000131d98, 0xdab701, 0x0, 0xc000131dc0)
        /home/rvagg/go/pkg/mod/github.com/ipld/go-ipld-prime@v0.14.5-0.20220214151842-d62ec3674f3a/codec/dagcbor/unmarshal.go:85 +0x125

I initially started this branch on an version of the code prior to the schema changes in #354 with the same error, so it's not just due the top-level keyed union introduced there.

I was also playing around with a variation of the example posted in ipld/go-ipld-prime#342 and was able to easily get panic: bindnode TODO: invalid key: "J" is not a field in type Foo if I just fed it a map with a misnamed field.

[rvagg]

I think that assemblerRepr() needs to be sensitive to _errorAssembler types and pass those through if it sees them; as it is it's brute forcing _assembler and failing.

I'm not sure why I can't manually reproduce that with a trivial example like the one at ipld/go-ipld-prime#342

[mvdan]

Thanks @rvagg - I've been able to repro by adding a few more tests to bindnode. I'll send a PR shortly.

[hannahhoward]

Thanks @mvdan & @rvagg -- it's probably important we solve as many issues as we can before we put this out there.

One other option we may want to look at -- one of the popular go CBOR libraries has a fuzzing utility that includes a corpus -- https://github.com/fxamacker/cbor-fuzz/tree/master/corpus -- we could just feed these to the decoder and see if we get a fail.

[hannahhoward]

AND we may also want to throw random bytes at the decoder, just to make sure we don't hit issues with REFMT's CBOR decoding.

[hannahhoward]

There is an interesting question here around responsibility -- at what point is just insuring that cbor decodes properly an IPLD prime / refmt problem and not a graphsync problem.

[mvdan]

FYI, I intend to tackle ipld/go-ipld-prime#357 next. I imagine I'll have picked most of the low-hanging fruit found via fuzzing in about a week's time from now.

[rvagg]

There is an interesting question here around responsibility

@hannahhoward yeah, I certainly don't think it's the responsibility of this library; I'm taking a personal interest because I know how new and raw everything is and significant potential for us shipping something that can easily DoS a Filecoin SP if we allow crashing. I'm anticipating @mvdan taking over this work, as per ipld/go-ipld-prime#357.

But what I was thinking, was that we should add a panic recover into the messaging layer, since we know how complex and new it is. I just didn't get around to doing that (and figuring out how to do it) in the initial round of changes. We should probably have a ticket for it though I suppose!

[rvagg]

Closing this because it's not something I think is particularly useful here, at least in its current state. Will leave the branch for now though.