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

fix panics in protoparse identified by oss-fuzz #412

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

jhump
Copy link
Owner

@jhump jhump commented Aug 5, 2021

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34232
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34238

Both bugs had the same root cause: unicode chars with very high code points that collided with the various constants for known token types.

So now the lexer is smart enough to limit the symbol type value for such runes to less than 256. The smallest constant is 57346, so plenty of headroom in between to avoid collisions. If it sees a symbol with a greater code point, it will now report an error "invalid character".

This should be fine since this handling is only for stand-alone runes/punctuation. Protobuf does not allow such symbols as part of identifiers nor as any allowed punctuation or operator. Unicode points like this are only valid in a proto source file inside of a string literal. So if we come across a lone rune with such a value (not in a string literal), then it is definitely a syntax error.

…t collided with the various constants for known token types
@jhump jhump merged commit 302c6b7 into master Aug 5, 2021
@jhump jhump deleted the jh/fix-oss-fuzz-bugs branch August 5, 2021 15:46
@jhump jhump mentioned this pull request Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant