pogs: Panic on invalid composite list #137
Comments
oncilla
added a commit
to oncilla/scion
that referenced
this issue
Apr 24, 2019
Catch panics when deserializing capnp messages using pogs. This could happen on malformed input, see capnproto/go-capnp#137
oncilla
added a commit
to scionproto/scion
that referenced
this issue
Apr 24, 2019
Catch panics when deserializing capnp messages using pogs. This could happen on malformed input, see capnproto/go-capnp#137
|
Hi @oncilla! Thanks for the report. Tracing the code, I suspect the offending line is this one: https://github.com/capnproto/go-capnproto2/blob/e1ae1f982d9908a41db464f02861a850a0880a5a/capn.go#L202 For composite lists, the number of elements is read from the tag word, which is formatted as a struct pointer and can have a negative number. I'd be happy to review a fix. |
oncilla
added a commit
to oncilla/go-capnproto2
that referenced
this issue
May 5, 2019
This PR prevents code from panicking and fixes some small errors during unpacking: Prevents malformed input for composite lists to cause the pogs library to panic. Make the `packed.Decoder` throw an error if the input is `0x00`. According to the spec, at least one more byte is required. Make the `packet.Decoder` throw an error if the input for the unpacked bytes (`0xFF` tag) does not contain enough unpacked bytes. fixes capnproto#137
|
@zombiezen I opened a PR to fix this issue and some smaller unpacking bugs (#139) |
zombiezen
pushed a commit
that referenced
this issue
May 5, 2019
This PR prevents code from panicking and fixes some small errors during unpacking: Prevents malformed input for composite lists to cause the pogs library to panic. Make the `packed.Decoder` throw an error if the input is `0x00`. According to the spec, at least one more byte is required. Make the `packet.Decoder` throw an error if the input for the unpacked bytes (`0xFF` tag) does not contain enough unpacked bytes. Additionally, update the go/gazelle rules and update the bazel version on CI. Fixes #137
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
During fuzzing go-capnproto2 with go-fuzz an error was found when parsing invalid composite lists.
(commit e1ae1f9)
When creating a
capnp.Listfrom acapnp.Ptr, theuint32valuePtr.lenOrCapis cast to aint32, possibly resulting in a negative value. During the validity check, the non-negativity of the list length is not considered, resulting in a panic in the pogs library extract.go:L261A minimal panicking example can be found here: https://github.com/Oncilla/go-capnproto2-fuzz
I'm happy to contribute a fix, but I am unsure what should be fixed here.
Casting vs IsValid check?
The text was updated successfully, but these errors were encountered: