Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBOR deserialization is not strict #2020

Closed
kostko opened this issue Aug 21, 2019 · 2 comments · Fixed by #3046
Closed

CBOR deserialization is not strict #2020

kostko opened this issue Aug 21, 2019 · 2 comments · Fixed by #3046
Labels
c:common Category: common libraries c:security Category: security issues

Comments

@kostko
Copy link
Member

kostko commented Aug 21, 2019

Currently the CBOR decoders do not enforce canonical serialization during decoding, resulting in issues when structures are round-tripped and stored in Merklized or otherwise authenticated data structures:

  • Bytes can be decoded from either null (in Go, but not in Rust), a list of uint8s, a byte array. When encoded they are encoded as a byte array.
  • Lists can be decoded from either null (in Go, but not in Rust) or a list.

If not checked properly this could easily transform subtle bugs into a denial of service vector. A specially crafted message that uses for example a list of uint8s on input instead of the normally used byte array could easily cause problems once those messages are serialized again and differ from the original bytes. The same goes for handling of nulls in list encoding which currently differs between Go and Rust.

We should either make sure that either:

  • structures are never round-tripped in this way (which is sometimes hard and/or introduces complexity) or
  • we make sure that our canonical CBOR decoder is strict during decoding (e.g., treating any non-canonical input serializations as malformed). In absence of support for this in codecs a hacky (and slow) solution is to perform a round-trip during decoding and checking if output is identical to input.

RFC 7049 defines "Strict Mode" in Section 3.10 which seems like something that we would need but as far as I see there is no support for something like this in either go-codec or serde. Both try to be maximally flexible during decoding which goes directly against this requirement.

@kostko kostko added c:common Category: common libraries c:security Category: security issues labels Aug 21, 2019
@Yawning
Copy link
Contributor

Yawning commented Aug 21, 2019

I'd vote for "structures are never round-tripped in this way" (or taking it further, "strutures are never round-tripped").

@Yawning
Copy link
Contributor

Yawning commented Oct 14, 2019

I tried adding a debug "force check if the structure round-trips" (#2258), but ran into lots of issues with our rust code not having the appropriate serde annotations for omitempty type behavior. So, another solution is needed (unless we want to go add annotations everywhere).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:common Category: common libraries c:security Category: security issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants