Validate the tag numbers when parsing. (#1725)
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.
diff --git a/objectivec/GPBCodedInputStream.m b/objectivec/GPBCodedInputStream.m
index acba715..2b578dd 100644
--- a/objectivec/GPBCodedInputStream.m
+++ b/objectivec/GPBCodedInputStream.m
@@ -227,7 +227,13 @@
state->lastTag = ReadRawVarint32(state);
if (state->lastTag == 0) {
// If we actually read zero, that's not a valid tag.
- RaiseException(GPBCodedInputStreamErrorInvalidTag, @"Last tag can't be 0");
+ RaiseException(GPBCodedInputStreamErrorInvalidTag,
+ @"A zero tag on the wire is invalid.");
+ }
+ // Tags have to include a valid wireformat, check that also.
+ if (!GPBWireFormatIsValidTag(state->lastTag)) {
+ RaiseException(GPBCodedInputStreamErrorInvalidTag,
+ @"Invalid wireformat in tag.");
}
return state->lastTag;
}
@@ -352,6 +358,7 @@
}
- (BOOL)skipField:(int32_t)tag {
+ NSAssert(GPBWireFormatIsValidTag(tag), @"Invalid tag");
switch (GPBWireFormatGetTagWireType(tag)) {
case GPBWireFormatVarint:
GPBCodedInputStreamReadInt32(&state_);
@@ -374,8 +381,6 @@
SkipRawData(&state_, sizeof(int32_t));
return YES;
}
- RaiseException(GPBCodedInputStreamErrorInvalidTag, nil);
- return NO;
}
- (void)skipMessage {
