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

Validate the tag numbers when parsing. #1725

Merged
merged 2 commits into from Jun 29, 2016

Conversation

thomasvl
Copy link
Contributor

There was a twist code path (that some times showed up due to what happened to
be in memory in failure cases), that would cast a bogus wire type into the
enum, and then fall through switch statements.

Resolve this by validating all wire types when parsing tags and throwing the
error at that point so it can't enter the system.

As added safety, stick in a few asserts for apis that get passed tags to ensure
they also are only seeing valid data.

Bonus: Tweak the parsing loop to skip some work when we get the end marker
(zero tag) instead of still looping through all the fields.

There was a twist code path (that some times showed up due to what happened to
be in memory in failure cases), that would cast a bogus wire type into the
enum, and then fall through switch statements.

Resolve this by validating all wire types when parsing tags and throwing the
error at that point so it can't enter the system.

As added safety, stick in a few asserts for apis that get passed tags to ensure
they also are only seeing valid data.

Bonus: Tweak the parsing loop to skip some work when we get the end marker
(zero tag) instead of still looping through all the fields.
@thomasvl thomasvl self-assigned this Jun 28, 2016
@@ -352,6 +358,7 @@ - (void)checkLastTagWas:(int32_t)value {
}

- (BOOL)skipField:(int32_t)tag {
NSAssert(GPBWireFormatIsValidTag(tag), @"Invalid tag?");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@sergiocampama
Copy link
Contributor

lgtm

@thomasvl thomasvl merged commit c18aa77 into protocolbuffers:master Jun 29, 2016
@thomasvl thomasvl deleted the validate_tags branch June 29, 2016 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants