fxamacker / cbor

CBOR codec (RFC 8949) with CBOR tags, Go struct tags (toarray, keyasint, omitempty), float64/32/16, big.Int, and fuzz tested billions of execs.
MIT License
718 stars 59 forks source link

Fixes required because both RFC 7049 Appendix A and Wikipedia violate RFC 7049 Section 2.3 #46

Closed x448 closed 4 years ago

x448 commented 4 years ago

Data validation should add this rule: Reject CBOR primitives (major type 7) with additional info 24 if value (next byte) is < 32.

BTW, always verify with RFC 7049 because there's incorrect info about this item even on reputable websites.

x448 commented 4 years ago

RFC 7049 Section 2.3. Floating-Point Numbers and Values with No Content

24 | Simple value (value 32..255 in following byte)

It also says in the same section:

As with all other major types, the 5-bit value 24 signifies a single- byte extension: it is followed by an additional byte to represent the simple value. (To minimize confusion, only the values 32 to 255 are used.)

This means RFC 7049 Appendix A contains an invalid example that violates RFC 7049 Section 2.3:

| simple(24) | 0xf818 |

After 6+ years, the RFC 7049 errata still doesn't mention this mistake so you can report it if you want.

Wikipedia is also wrong because it's interpretation coincidentally matches the incorrect example from RFC 7049 Appendix A instead of the correct text from RFC 7049 Section 2.3.

fxamacker commented 4 years ago

Thanks for finding the root cause. I just filed a RFC errata for this issue.

I have hundreds more corpus files compared to what is shared online because fuzzing started before v0.1 was put on github.

Thanks again :+1:

cabo commented 4 years ago

Thank you for filing this errata report.

We fixed this in the 7049bis document (which is updating the current RFC with some significant editorial improvements, as well as fixing the errata, currently in working-group last call) in revision f3bde9d: https://github.com/cbor-wg/CBORbis/commit/f3bde9d . Unfortunately, at the time it didn’t occur to us to also supply an errata report, which should have been done.

Maybe I can use this unfortunate event as an opportunity to ask that you have a good read of https://tools.ietf.org/html/draft-ietf-cbor-7049bis-09 and submit any problems you have with the updated text before the working group last call completes on Thursday, 12 December? You can tell us either at cbor@ietf.org or at https://github.com/cbor-wg/CBORbis/issues . Note that there are a few issues outstanding, in particular https://github.com/cbor-wg/CBORbis/issues/63 — which actually will have a substantive effect, so you may have an opinion on it; please feel free to comment.

fxamacker commented 4 years ago

@cabo thanks for confirming. I went to update wikipedia but it looks like someone beat me to it! :smile:

7049bis has some really nice editorial improvements, such as section 4.2 (Deterministically Encoded CBOR). I'm looking forward to read more during the holidays.

I found a couple of potential issues in 7049bis. I need more time to double check before bringing them to your attention.

Thanks again for your reply.

x448 commented 4 years ago

Thanks Carsten! I agree with Faye, 7049bis is a very welcome improvement over 7049 from what I've read so far. I regret not finding it sooner and making suggestions/contributions earlier. I have suggestions that wouldn't change 7049bis rules but would make it easier to create generic CBOR libraries with 7049bis -- but December 12 is so close.