[ObjC] Enforce bytes and string field size limits.
PiperOrigin-RevId: 509863774
diff --git a/objectivec/GPBCodedInputStream.m b/objectivec/GPBCodedInputStream.m
index ad5c75e..9acf6f1 100644
--- a/objectivec/GPBCodedInputStream.m
+++ b/objectivec/GPBCodedInputStream.m
@@ -51,6 +51,10 @@
// int CodedInputStream::default_recursion_limit_ = 100;
static const NSUInteger kDefaultRecursionLimit = 100;
+// Bytes and Strings have a max size of 2GB.
+// https://protobuf.dev/programming-guides/encoding/#cheat-sheet
+static const uint32_t kMaxFieldSize = 0x7fffffff;
+
static void RaiseException(NSInteger code, NSString *reason) {
NSDictionary *errorInfo = nil;
if ([reason length]) {
@@ -223,14 +227,20 @@
}
NSString *GPBCodedInputStreamReadRetainedString(GPBCodedInputStreamState *state) {
- int32_t size = ReadRawVarint32(state);
+ uint64_t size = GPBCodedInputStreamReadUInt64(state);
+ if (size > kMaxFieldSize) {
+ // TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
+ // so reuse an existing one.
+ RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
+ }
+ NSUInteger ns_size = (NSUInteger)size;
NSString *result;
if (size == 0) {
result = @"";
} else {
CheckSize(state, size);
result = [[NSString alloc] initWithBytes:&state->bytes[state->bufferPos]
- length:size
+ length:ns_size
encoding:NSUTF8StringEncoding];
state->bufferPos += size;
if (!result) {
@@ -246,21 +256,31 @@
}
NSData *GPBCodedInputStreamReadRetainedBytes(GPBCodedInputStreamState *state) {
- int32_t size = ReadRawVarint32(state);
- if (size < 0) return nil;
+ uint64_t size = GPBCodedInputStreamReadUInt64(state);
+ if (size > kMaxFieldSize) {
+ // TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
+ // so reuse an existing one.
+ RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
+ }
+ NSUInteger ns_size = (NSUInteger)size;
CheckSize(state, size);
- NSData *result = [[NSData alloc] initWithBytes:state->bytes + state->bufferPos length:size];
+ NSData *result = [[NSData alloc] initWithBytes:state->bytes + state->bufferPos length:ns_size];
state->bufferPos += size;
return result;
}
NSData *GPBCodedInputStreamReadRetainedBytesNoCopy(GPBCodedInputStreamState *state) {
- int32_t size = ReadRawVarint32(state);
- if (size < 0) return nil;
+ uint64_t size = GPBCodedInputStreamReadUInt64(state);
+ if (size > kMaxFieldSize) {
+ // TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
+ // so reuse an existing one.
+ RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
+ }
+ NSUInteger ns_size = (NSUInteger)size;
CheckSize(state, size);
// Cast is safe because freeWhenDone is NO.
NSData *result = [[NSData alloc] initWithBytesNoCopy:(void *)(state->bytes + state->bufferPos)
- length:size
+ length:ns_size
freeWhenDone:NO];
state->bufferPos += size;
return result;
diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index f414570..a06bb6f 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -2003,9 +2003,6 @@
return;
}
NSData *data = GPBCodedInputStreamReadRetainedBytesNoCopy(state);
- if (data == nil) {
- return;
- }
[self mergeFromData:data extensionRegistry:extensionRegistry];
[data release];
}
diff --git a/objectivec/Tests/GPBCodedInputStreamTests.m b/objectivec/Tests/GPBCodedInputStreamTests.m
index 0504062..0d0d327 100644
--- a/objectivec/Tests/GPBCodedInputStreamTests.m
+++ b/objectivec/Tests/GPBCodedInputStreamTests.m
@@ -28,6 +28,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#import <Foundation/Foundation.h>
#import "GPBTestUtilities.h"
#import "GPBCodedInputStream.h"
@@ -370,10 +371,23 @@
@"should throw a GPBCodedInputStreamException exception ");
}
-- (void)testBytesWithNegativeSize {
- NSData* data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F);
+- (void)testBytesOver2GB {
+ NSData* data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F, 0x01, 0x02, 0x03); // don't need all the bytes
GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data];
- XCTAssertNil([input readBytes]);
+ @try {
+ __unused NSData* result = [input readBytes];
+ XCTFail(@"Should have thrown");
+ } @catch (NSException* anException) {
+ // Ensure the correct error within the exception.
+ XCTAssertTrue([anException isKindOfClass:[NSException class]]);
+ XCTAssertEqualObjects(anException.name, GPBCodedInputStreamException);
+ NSDictionary* userInfo = anException.userInfo;
+ XCTAssertNotNil(userInfo);
+ NSError* err = userInfo[GPBCodedInputStreamUnderlyingErrorKey];
+ XCTAssertNotNil(err);
+ XCTAssertEqualObjects(err.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(err.code, GPBCodedInputStreamErrorInvalidSize);
+ }
}
// Verifies fix for b/10315336.
diff --git a/objectivec/Tests/GPBMessageTests+Serialization.m b/objectivec/Tests/GPBMessageTests+Serialization.m
index 3c2381c..5a7dc16 100644
--- a/objectivec/Tests/GPBMessageTests+Serialization.m
+++ b/objectivec/Tests/GPBMessageTests+Serialization.m
@@ -1266,12 +1266,17 @@
XCTAssertEqual(error.code, GPBCodedInputStreamErrorRecursionDepthExceeded);
}
-- (void)testParseDelimitedDataWithNegativeSize {
- NSData *data = DataFromCStr("\xFF\xFF\xFF\xFF\x0F");
+- (void)testParseDelimitedDataOver2GB {
+ NSData *data = DataFromCStr("\xFF\xFF\xFF\xFF\x0F\x01\x02\0x3"); // Don't need all the bytes
GPBCodedInputStream *input = [GPBCodedInputStream streamWithData:data];
NSError *error;
- [GPBMessage parseDelimitedFromCodedInputStream:input extensionRegistry:nil error:&error];
- XCTAssertNil(error);
+ GPBMessage *result = [GPBMessage parseDelimitedFromCodedInputStream:input
+ extensionRegistry:nil
+ error:&error];
+ XCTAssertNil(result);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidSize);
}
#ifdef DEBUG
