Skip to content

Commit 8f89e0d

Browse files
[ObjC] Enforce bytes and string field size limits.
PiperOrigin-RevId: 509863774
1 parent 0c21b63 commit 8f89e0d

File tree

4 files changed

+54
-18
lines changed

4 files changed

+54
-18
lines changed

objectivec/GPBCodedInputStream.m

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151
// int CodedInputStream::default_recursion_limit_ = 100;
5252
static const NSUInteger kDefaultRecursionLimit = 100;
5353

54+
// Bytes and Strings have a max size of 2GB.
55+
// https://protobuf.dev/programming-guides/encoding/#cheat-sheet
56+
static const uint32_t kMaxFieldSize = 0x7fffffff;
57+
5458
static void RaiseException(NSInteger code, NSString *reason) {
5559
NSDictionary *errorInfo = nil;
5660
if ([reason length]) {
@@ -223,14 +227,20 @@ int32_t GPBCodedInputStreamReadTag(GPBCodedInputStreamState *state) {
223227
}
224228

225229
NSString *GPBCodedInputStreamReadRetainedString(GPBCodedInputStreamState *state) {
226-
int32_t size = ReadRawVarint32(state);
230+
uint64_t size = GPBCodedInputStreamReadUInt64(state);
231+
if (size > kMaxFieldSize) {
232+
// TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
233+
// so reuse an existing one.
234+
RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
235+
}
236+
NSUInteger ns_size = (NSUInteger)size;
227237
NSString *result;
228238
if (size == 0) {
229239
result = @"";
230240
} else {
231241
CheckSize(state, size);
232242
result = [[NSString alloc] initWithBytes:&state->bytes[state->bufferPos]
233-
length:size
243+
length:ns_size
234244
encoding:NSUTF8StringEncoding];
235245
state->bufferPos += size;
236246
if (!result) {
@@ -246,21 +256,31 @@ int32_t GPBCodedInputStreamReadTag(GPBCodedInputStreamState *state) {
246256
}
247257

248258
NSData *GPBCodedInputStreamReadRetainedBytes(GPBCodedInputStreamState *state) {
249-
int32_t size = ReadRawVarint32(state);
250-
if (size < 0) return nil;
259+
uint64_t size = GPBCodedInputStreamReadUInt64(state);
260+
if (size > kMaxFieldSize) {
261+
// TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
262+
// so reuse an existing one.
263+
RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
264+
}
265+
NSUInteger ns_size = (NSUInteger)size;
251266
CheckSize(state, size);
252-
NSData *result = [[NSData alloc] initWithBytes:state->bytes + state->bufferPos length:size];
267+
NSData *result = [[NSData alloc] initWithBytes:state->bytes + state->bufferPos length:ns_size];
253268
state->bufferPos += size;
254269
return result;
255270
}
256271

257272
NSData *GPBCodedInputStreamReadRetainedBytesNoCopy(GPBCodedInputStreamState *state) {
258-
int32_t size = ReadRawVarint32(state);
259-
if (size < 0) return nil;
273+
uint64_t size = GPBCodedInputStreamReadUInt64(state);
274+
if (size > kMaxFieldSize) {
275+
// TODO(thomasvl): Maybe a different error code for this, but adding one is a breaking change
276+
// so reuse an existing one.
277+
RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
278+
}
279+
NSUInteger ns_size = (NSUInteger)size;
260280
CheckSize(state, size);
261281
// Cast is safe because freeWhenDone is NO.
262282
NSData *result = [[NSData alloc] initWithBytesNoCopy:(void *)(state->bytes + state->bufferPos)
263-
length:size
283+
length:ns_size
264284
freeWhenDone:NO];
265285
state->bufferPos += size;
266286
return result;

objectivec/GPBMessage.m

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2003,9 +2003,6 @@ - (void)mergeDelimitedFromCodedInputStream:(GPBCodedInputStream *)input
20032003
return;
20042004
}
20052005
NSData *data = GPBCodedInputStreamReadRetainedBytesNoCopy(state);
2006-
if (data == nil) {
2007-
return;
2008-
}
20092006
[self mergeFromData:data extensionRegistry:extensionRegistry];
20102007
[data release];
20112008
}

objectivec/Tests/GPBCodedInputStreamTests.m

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
2929
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
3030

31+
#import <Foundation/Foundation.h>
3132
#import "GPBTestUtilities.h"
3233

3334
#import "GPBCodedInputStream.h"
@@ -370,10 +371,23 @@ - (void)testInvalidGroupEndTagThrows {
370371
@"should throw a GPBCodedInputStreamException exception ");
371372
}
372373

373-
- (void)testBytesWithNegativeSize {
374-
NSData* data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F);
374+
- (void)testBytesOver2GB {
375+
NSData* data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F, 0x01, 0x02, 0x03); // don't need all the bytes
375376
GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data];
376-
XCTAssertNil([input readBytes]);
377+
@try {
378+
__unused NSData* result = [input readBytes];
379+
XCTFail(@"Should have thrown");
380+
} @catch (NSException* anException) {
381+
// Ensure the correct error within the exception.
382+
XCTAssertTrue([anException isKindOfClass:[NSException class]]);
383+
XCTAssertEqualObjects(anException.name, GPBCodedInputStreamException);
384+
NSDictionary* userInfo = anException.userInfo;
385+
XCTAssertNotNil(userInfo);
386+
NSError* err = userInfo[GPBCodedInputStreamUnderlyingErrorKey];
387+
XCTAssertNotNil(err);
388+
XCTAssertEqualObjects(err.domain, GPBCodedInputStreamErrorDomain);
389+
XCTAssertEqual(err.code, GPBCodedInputStreamErrorInvalidSize);
390+
}
377391
}
378392

379393
// Verifies fix for b/10315336.

objectivec/Tests/GPBMessageTests+Serialization.m

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,12 +1266,17 @@ - (void)testErrorRecursionDepthReached {
12661266
XCTAssertEqual(error.code, GPBCodedInputStreamErrorRecursionDepthExceeded);
12671267
}
12681268

1269-
- (void)testParseDelimitedDataWithNegativeSize {
1270-
NSData *data = DataFromCStr("\xFF\xFF\xFF\xFF\x0F");
1269+
- (void)testParseDelimitedDataOver2GB {
1270+
NSData *data = DataFromCStr("\xFF\xFF\xFF\xFF\x0F\x01\x02\0x3"); // Don't need all the bytes
12711271
GPBCodedInputStream *input = [GPBCodedInputStream streamWithData:data];
12721272
NSError *error;
1273-
[GPBMessage parseDelimitedFromCodedInputStream:input extensionRegistry:nil error:&error];
1274-
XCTAssertNil(error);
1273+
GPBMessage *result = [GPBMessage parseDelimitedFromCodedInputStream:input
1274+
extensionRegistry:nil
1275+
error:&error];
1276+
XCTAssertNil(result);
1277+
XCTAssertNotNil(error);
1278+
XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
1279+
XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidSize);
12751280
}
12761281

12771282
#ifdef DEBUG

0 commit comments

Comments
 (0)