From 10ad36b27eeab917bc647b0fd2e6fe9fc14986bc Mon Sep 17 00:00:00 2001 From: Jay Patel Date: Sat, 22 Aug 2026 14:58:42 -0400 Subject: [PATCH 1/3] fix(crashlytics): skip binary image records with a non-numeric base or size --- Crashlytics/CHANGELOG.md | 4 ++++ .../Crashlytics/Models/FIRCLSSymbolResolver.m | 12 ++++-------- .../binary_images_with_string_base_entry.clsrecord | 6 ++++++ Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m | 9 +++++++++ 4 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 Crashlytics/UnitTests/Data/binary_images_with_string_base_entry.clsrecord diff --git a/Crashlytics/CHANGELOG.md b/Crashlytics/CHANGELOG.md index 70a399591e3..058774d874a 100644 --- a/Crashlytics/CHANGELOG.md +++ b/Crashlytics/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased +- [fixed] Fixed a crash when processing a report whose binary image records + contain a non-numeric `base` or `size`. (#16519) + # 12.18.0 - [removed] Removes unused integration with the now deprecated ObjC MetricKit API. diff --git a/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m b/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m index 8ce74da5690..a43a228caa5 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m +++ b/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m @@ -60,14 +60,10 @@ - (BOOL)loadBinaryImagesFromFile:(NSString*)path { continue; } - // This does happen occasionally and causes a crash. I'm really not sure there - // is anything sane we can do in this case. - if (![details objectForKey:@"base"] || ![details objectForKey:@"size"]) { - continue; - } - - if ([details objectForKey:@"base"] == (id)[NSNull null] || - [details objectForKey:@"size"] == (id)[NSNull null]) { + // base and size come from JSON and are occasionally missing, NSNull, or some other + // non-numeric type, which would crash the sort comparator below. + if (![[details objectForKey:@"base"] isKindOfClass:[NSNumber class]] || + ![[details objectForKey:@"size"] isKindOfClass:[NSNumber class]]) { continue; } diff --git a/Crashlytics/UnitTests/Data/binary_images_with_string_base_entry.clsrecord b/Crashlytics/UnitTests/Data/binary_images_with_string_base_entry.clsrecord new file mode 100644 index 00000000000..881c05441ab --- /dev/null +++ b/Crashlytics/UnitTests/Data/binary_images_with_string_base_entry.clsrecord @@ -0,0 +1,6 @@ +{"load":{"path":"/Users/mmassicotte/Library/Developer/CoreSimulator/Devices/5FC72D9A-2048-482A-9A79-2CA9F1FC18FB/data/Containers/Bundle/Application/788D77C9-161A-4021-9774-7E3C9393AD45/CrashTest-iOS.app/CrashTest-iOS","uuid":"e07d35c9f99a3a82a3638faf8775eccc","base":4389027840,"size":598016}} +{"load":{"path":"/Applications/Xcode 6.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator8.2.sdk/Developer/Library/PrivateFrameworks/DTDDISupport.framework/libViewDebuggerSupport.dylib","bundle_id":null,"build_version":null,"display_version":null,"uuid":"7f81304e1e243c9c928a1cae4217e116","base":4390633472,"size":24576}} +{"load":{"path":"/Applications/Xcode 6.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/Frameworks/AdSupport.framework/AdSupport","bundle_id":null,"build_version":null,"display_version":null,"uuid":"be521a1e2d003fa8a1e84deb6c3ebfe5","base":4390678528,"size":4096}} +{"load":{"path":"/Applications/Xcode 6.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/lib/libz.1.dylib","uuid":"7cca2d44f20a35a59b7cb026a188f4f5","base":4390699008,"size":77824}} +{"load":{"path":"/Applications/Xcode 6.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics","bundle_id":null,"build_version":null,"display_version":null,"uuid":"9a455d304f0735be8a03253c5b17aecd","base":4390797312,"size":1884160}} +{"load":{"path":"/Applications/Xcode 6.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/Frameworks/UIKit.framework/UIKit","bundle_id":null,"build_version":null,"display_version":null,"uuid":"cdd3270b77223f4f933368746205a9cf","base":"0x105a3c000","size":11964416}} diff --git a/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m b/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m index 71a163f0b57..9cf0c97f78d 100644 --- a/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m +++ b/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m @@ -68,4 +68,13 @@ - (void)testLoadingBinaryImagesWithMissingBaseValue { XCTAssert([resolver loadBinaryImagesFromFile:binaryImagePath]); } +- (void)testLoadingBinaryImagesWithStringBaseValue { + FIRCLSSymbolResolver* resolver = [[FIRCLSSymbolResolver alloc] init]; + + NSString* binaryImagePath = + [self pathForResource:@"binary_images_with_string_base_entry.clsrecord"]; + + XCTAssert([resolver loadBinaryImagesFromFile:binaryImagePath]); +} + @end From 9455f4e65bbaa39dbc89fbbac39d2d3e1a737d27 Mon Sep 17 00:00:00 2001 From: Jay Patel Date: Sat, 22 Aug 2026 19:10:14 -0400 Subject: [PATCH 2/3] refactor(crashlytics): read base and size into locals Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m b/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m index a43a228caa5..a4b9f835342 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m +++ b/Crashlytics/Crashlytics/Models/FIRCLSSymbolResolver.m @@ -62,8 +62,9 @@ - (BOOL)loadBinaryImagesFromFile:(NSString*)path { // base and size come from JSON and are occasionally missing, NSNull, or some other // non-numeric type, which would crash the sort comparator below. - if (![[details objectForKey:@"base"] isKindOfClass:[NSNumber class]] || - ![[details objectForKey:@"size"] isKindOfClass:[NSNumber class]]) { + id base = [details objectForKey:@"base"]; + id size = [details objectForKey:@"size"]; + if (![base isKindOfClass:[NSNumber class]] || ![size isKindOfClass:[NSNumber class]]) { continue; } From f96b9166b71a8deb1ea26dbf84f83559354b896b Mon Sep 17 00:00:00 2001 From: Jay Patel Date: Wed, 26 Aug 2026 09:53:04 -0400 Subject: [PATCH 3/3] test(crashlytics): assert the non-numeric record is skipped --- Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m b/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m index 9cf0c97f78d..ab287470dfd 100644 --- a/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m +++ b/Crashlytics/UnitTests/FIRCLSSymbolResolverTests.m @@ -16,6 +16,12 @@ #import +// -loadedBinaryImageForPC: is internal to FIRCLSSymbolResolver, but it is the only way to +// observe which records survived filtering. +@interface FIRCLSSymbolResolver (Testing) +- (NSDictionary*)loadedBinaryImageForPC:(uintptr_t)pc; +@end + @interface FIRCLSSymbolResolverTests : XCTestCase @end @@ -75,6 +81,11 @@ - (void)testLoadingBinaryImagesWithStringBaseValue { [self pathForResource:@"binary_images_with_string_base_entry.clsrecord"]; XCTAssert([resolver loadBinaryImagesFromFile:binaryImagePath]); + + // The record with the string base covers 0x105a3c000 and up, it should have been skipped. + XCTAssertNil([resolver loadedBinaryImageForPC:4395000000]); + + XCTAssertNotNil([resolver loadedBinaryImageForPC:4389027840]); } @end