From 6b7cf78a74d0dd70f9ed211a4bf5e154cddc5d8f Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 21 Aug 2026 17:33:32 -0400 Subject: [PATCH 1/2] Update firebase_performance_swizzle_denylist to also prevent objects and proxies from being swizzled --- FirebasePerformance/CHANGELOG.md | 3 + .../Sources/Instrumentation/FPRInstrument.m | 31 +++++++--- .../FPRNSURLSessionDelegateInstrument.m | 9 +++ .../Network/FPRNSURLSessionInstrument.m | 4 ++ .../Tests/Unit/FPRInstrumentTest.m | 40 +++++++++++++ .../FPRNSURLConnectionInstrumentTest.m | 20 +++++++ .../FPRNSURLSessionInstrumentTest.m | 58 +++++++++++++++++++ 7 files changed, 158 insertions(+), 7 deletions(-) diff --git a/FirebasePerformance/CHANGELOG.md b/FirebasePerformance/CHANGELOG.md index 9414b3504f5..bf1fe1279e8 100644 --- a/FirebasePerformance/CHANGELOG.md +++ b/FirebasePerformance/CHANGELOG.md @@ -3,6 +3,9 @@ falsely classifying 60 FPS frames as slow on ProMotion devices, while preserving dynamic frame rate support for tvOS. - [fixed] Fixed a crash caused due to ISA swizzling weak ivars. (#16469) + while preserving dynamic frame rate support for tvOS. (#10220) +- [fixed] Honor `firebase_performance_swizzle_denylist` when registering objects + and proxies for swizzling. (#16469) # 12.16.0 - [fixed] Fixed a crash in `FPRMemoryGaugeCollector` by collecting memory usage diff --git a/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m b/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m index 32ef46e8dbd..2fc06ec5271 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#import + #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument.h" #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h" @@ -49,22 +51,37 @@ - (void)registerInstrumentors { } - (BOOL)isObjectInstrumentable:(id)object { - if ([object isKindOfClass:[NSOperation class]]) { + if (!object || (![object isProxy] && [object isKindOfClass:[NSOperation class]])) { + return NO; + } + Class objectClass = [object isProxy] ? object_getClass(object) : [object class]; + return [self isClassInstrumentable:objectClass]; +} + +- (BOOL)isClassInstrumentable:(Class)aClass { + NSString *className = NSStringFromClass(aClass); + if (!className) { + // If the className is nil, it should be a no-op. return NO; } + + if ([[FPRConfigurations sharedInstance].swizzleClassDenylist containsObject:className]) { + FPRLogInfo(kFPRSwizzleClassDenylisted, + @"Skipped swizzling %@ because it is listed in " + @"firebase_performance_swizzle_denylist.", + className); + return NO; + } + return YES; } - (BOOL)registerClassInstrumentor:(FPRClassInstrumentor *)instrumentor { @synchronized(self) { - NSString *className = NSStringFromClass(instrumentor.instrumentedClass); - if ([[FPRConfigurations sharedInstance].swizzleClassDenylist containsObject:className]) { - FPRLogInfo(kFPRSwizzleClassDenylisted, - @"Skipped swizzling %@ because it is listed in " - @"firebase_performance_swizzle_denylist.", - className); + if (![self isClassInstrumentable:instrumentor.instrumentedClass]) { return NO; } + if ([_instrumentedClasses containsObject:instrumentor.instrumentedClass] || [instrumentor.instrumentedClass instancesRespondToSelector:@selector(gul_class)]) { return NO; diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index ae4a343d27b..0a0bc19950f 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -237,9 +237,14 @@ - (void)registerClass:(Class)aClass { - (void)registerObject:(id)object { dispatch_sync(GetInstrumentationQueue(), ^{ + if (![self isObjectInstrumentable:object]) { + return; + } + if ([object respondsToSelector:@selector(gul_class)]) { return; } + FPRObjectInstrumentor *instrumentor = [[FPRObjectInstrumentor alloc] initWithObject:object]; // Register the non-swizzled versions of these methods. @@ -266,6 +271,10 @@ - (void)registerObject:(id)object { } - (void)registerProxy:(id)proxy { + if (![self isObjectInstrumentable:proxy]) { + return; + } + [FPRProxyObjectHelper registerProxyObject:proxy forProtocol:@protocol(NSURLSessionDelegate) varFoundHandler:^(id ivar) { diff --git a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m index bf50974bfdd..ffdc951d3c6 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m @@ -708,6 +708,10 @@ - (void)registerInstrumentorForClass:(Class)aClass { } - (void)registerProxyObject:(id)proxy { + if (![self isObjectInstrumentable:proxy]) { + return; + } + [FPRProxyObjectHelper registerProxyObject:proxy forSuperclass:[NSURLSession class] varFoundHandler:^(id ivar) { diff --git a/FirebasePerformance/Tests/Unit/FPRInstrumentTest.m b/FirebasePerformance/Tests/Unit/FPRInstrumentTest.m index 245d0ac0d47..c2508fe2898 100644 --- a/FirebasePerformance/Tests/Unit/FPRInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/FPRInstrumentTest.m @@ -14,6 +14,7 @@ #import +#import "FirebasePerformance/Sources/Configurations/FPRConfigurations.h" #import "FirebasePerformance/Sources/Instrumentation/FPRClassInstrumentor.h" #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument.h" #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h" @@ -63,6 +64,45 @@ - (void)testRegisterAlreadyRegisteredClassInstrumentor { [instrument deregisterInstrumentors]; } +- (void)testIsObjectInstrumentableWithValidObject { + FPRInstrument *instrument = [[FPRInstrument alloc] init]; + NSObject *object = [[NSObject alloc] init]; + XCTAssertTrue([instrument isObjectInstrumentable:object]); +} + +- (void)testIsObjectInstrumentableWithNSOperation { + FPRInstrument *instrument = [[FPRInstrument alloc] init]; + NSOperation *operation = [[NSOperation alloc] init]; + XCTAssertFalse([instrument isObjectInstrumentable:operation]); +} + +- (void)testIsObjectInstrumentableWithNil { + FPRInstrument *instrument = [[FPRInstrument alloc] init]; + XCTAssertFalse([instrument isObjectInstrumentable:nil]); +} + +- (void)testIsObjectInstrumentableWithDenylistedClass { + FPRInstrument *instrument = [[FPRInstrument alloc] init]; + NSObject *object = [[NSObject alloc] init]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] andReturn:@[ NSStringFromClass([NSObject class]) ]] swizzleClassDenylist]; + XCTAssertFalse([instrument isObjectInstrumentable:object]); + [mockConfig stopMocking]; +} + +- (void)testRegisterClassInstrumentorWithDenylistedClass { + FPRInstrument *instrument = [[FPRInstrument alloc] init]; + FPRClassInstrumentor *instrumentor = + [[FPRClassInstrumentor alloc] initWithClass:[NSObject class]]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] andReturn:@[ NSStringFromClass([NSObject class]) ]] swizzleClassDenylist]; + BOOL success = [instrument registerClassInstrumentor:instrumentor]; + XCTAssertFalse(success); + XCTAssertEqual(instrument.classInstrumentors.count, 0); + XCTAssertEqual(instrument.instrumentedClasses.count, 0); + [mockConfig stopMocking]; +} + #pragma mark - Unswizzle based tests #if !SWIFT_PACKAGE diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLConnectionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLConnectionInstrumentTest.m index bb3e3520f39..024a743f720 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLConnectionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLConnectionInstrumentTest.m @@ -18,11 +18,13 @@ #import "FirebasePerformance/Tests/Unit/Instruments/FPRNSURLConnectionInstrumentTestDelegates.h" +#import #import #import "FirebasePerformance/Sources/Configurations/FPRConfigurations+Private.h" #import "FirebasePerformance/Sources/Configurations/FPRConfigurations.h" #import "FirebasePerformance/Sources/FPRClient.h" +#import "FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.h" #import "FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLConnectionInstrument.h" #import "FirebasePerformance/Sources/Public/FirebasePerformance/FIRPerformance.h" @@ -168,6 +170,24 @@ - (void)testInitWithOperationRequestDelegate { [instrument deregisterInstrumentors]; } +/** Tests that registerObject: skips swizzling when the delegate class is in swizzleClassDenylist. + */ +- (void)testRegisterObjectSkippedWhenClassIsDenylisted { + FPRNSURLConnectionCompleteTestDelegate *delegate = + [[FPRNSURLConnectionCompleteTestDelegate alloc] init]; + FPRNSURLConnectionDelegateInstrument *delegateInstrument = + [[FPRNSURLConnectionDelegateInstrument alloc] init]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] + andReturn:@[ NSStringFromClass([FPRNSURLConnectionCompleteTestDelegate class]) ]] + swizzleClassDenylist]; + + [delegateInstrument registerObject:delegate]; + XCTAssertFalse([delegate respondsToSelector:@selector(gul_class)]); + + [mockConfig stopMocking]; +} + /** Tests calling -initWithRequest:delegate: is wrapped and calls through with nil delegate. */ - (void)testInitWithRequestAndNilDelegate { FPRNSURLConnectionInstrument *instrument = [[FPRNSURLConnectionInstrument alloc] init]; diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 173b064b33c..8b9195d7b6d 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -18,6 +18,7 @@ #import "FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h" +#import #import #import @@ -697,6 +698,63 @@ - (void)testWeakProxyDelegateSkipsSwizzlingDelegate { [instrument deregisterInstrumentors]; } +/** Tests that registerObject: skips swizzling when the delegate class is in swizzleClassDenylist. + */ +- (void)testRegisterObjectSkippedWhenClassIsDenylisted { + FPRNSURLSessionTestDelegate *delegate = [[FPRNSURLSessionTestDelegate alloc] init]; + FPRNSURLSessionDelegateInstrument *delegateInstrument = + [[FPRNSURLSessionDelegateInstrument alloc] init]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] andReturn:@[ NSStringFromClass([FPRNSURLSessionTestDelegate class]) ]] + swizzleClassDenylist]; + + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + [delegateInstrument registerObject:delegate]; + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + XCTAssertFalse([delegate respondsToSelector:@selector(gul_class)]); + + [mockConfig stopMocking]; +} + +/** Tests that registerProxy: skips swizzling when the proxy class is in swizzleClassDenylist. */ +- (void)testRegisterProxySkippedWhenProxyClassIsDenylisted { + FPRNSURLSessionTestDelegate *delegate = [[FPRNSURLSessionTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + FPRNSURLSessionDelegateInstrument *delegateInstrument = + [[FPRNSURLSessionDelegateInstrument alloc] init]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] andReturn:@[ NSStringFromClass(object_getClass(proxyDelegate)) ]] + swizzleClassDenylist]; + + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + [delegateInstrument registerProxy:proxyDelegate]; + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + XCTAssertEqual(delegateInstrument.classInstrumentors.count, 0); + + [mockConfig stopMocking]; +} + +/** Tests that registerProxy: skips the wrapped delegate when its class is in swizzleClassDenylist. + */ +- (void)testRegisterProxySkipsWrappedDelegateWhenWrappedClassIsDenylisted { + FPRNSURLSessionTestDelegate *delegate = [[FPRNSURLSessionTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + FPRNSURLSessionDelegateInstrument *delegateInstrument = + [[FPRNSURLSessionDelegateInstrument alloc] init]; + id mockConfig = [OCMockObject partialMockForObject:[FPRConfigurations sharedInstance]]; + [[[mockConfig stub] andReturn:@[ NSStringFromClass([FPRNSURLSessionTestDelegate class]) ]] + swizzleClassDenylist]; + + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + [delegateInstrument registerProxy:proxyDelegate]; + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + XCTAssertEqual(delegateInstrument.classInstrumentors.count, 0); + + [mockConfig stopMocking]; +} + /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testProxyDelegateURLSessionTaskDidCompleteWithError { [self.testServer stop]; From f2aaed9b34aed709274fd96589f93f4d27c35dd7 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 24 Aug 2026 11:22:42 -0400 Subject: [PATCH 2/2] Refactor FPRUIViewControllerInstrument and FPRNSURLConnectionInstrument to not crash / swizzle if it's in the denylist. --- FirebasePerformance/CHANGELOG.md | 5 ++--- .../Sources/Instrumentation/FPRInstrument.m | 1 + .../Sources/Instrumentation/FPRInstrument_Private.h | 8 ++++++++ .../Network/FPRNSURLConnectionInstrument.m | 6 ++++++ .../Instrumentation/UIKit/FPRUIViewControllerInstrument.m | 6 ++++++ 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/FirebasePerformance/CHANGELOG.md b/FirebasePerformance/CHANGELOG.md index bf1fe1279e8..d761ad70c02 100644 --- a/FirebasePerformance/CHANGELOG.md +++ b/FirebasePerformance/CHANGELOG.md @@ -3,9 +3,8 @@ falsely classifying 60 FPS frames as slow on ProMotion devices, while preserving dynamic frame rate support for tvOS. - [fixed] Fixed a crash caused due to ISA swizzling weak ivars. (#16469) - while preserving dynamic frame rate support for tvOS. (#10220) -- [fixed] Honor `firebase_performance_swizzle_denylist` when registering objects - and proxies for swizzling. (#16469) +- [fixed] Honor `firebase_performance_swizzle_denylist` when registering objects, + proxies and additional classes for swizzling. (#16469) # 12.16.0 - [fixed] Fixed a crash in `FPRMemoryGaugeCollector` by collecting memory usage diff --git a/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m b/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m index 2fc06ec5271..fd2cb02705f 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/FPRInstrument.m @@ -78,6 +78,7 @@ - (BOOL)isClassInstrumentable:(Class)aClass { - (BOOL)registerClassInstrumentor:(FPRClassInstrumentor *)instrumentor { @synchronized(self) { + // Check if it's in the denylist. if (![self isClassInstrumentable:instrumentor.instrumentedClass]) { return NO; } diff --git a/FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h b/FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h index 27bfedb8e9e..d0d4638e67c 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h +++ b/FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h @@ -18,6 +18,14 @@ NS_ASSUME_NONNULL_BEGIN @interface FPRInstrument () +/** Verifies whether the class should be instrumented. The decision is based on + * `FPRConfigurations`. + * + * @param aClass The class to verify if it's in the denylist. + * @return NO if the class is in the denylist, YES otherwise. + */ +- (BOOL)isClassInstrumentable:(Class)aClass; + /** Registers an instrumentor for a class. Should be called by subclasses. * * @param instrumentor The instrumentor to register. diff --git a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLConnectionInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLConnectionInstrument.m index a3534edc1cb..0484f2e2a22 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLConnectionInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLConnectionInstrument.m @@ -209,6 +209,12 @@ - (void)dealloc { - (void)registerInstrumentors { dispatch_sync(GetInstrumentationQueue(), ^{ + // Check if it's in the denylist. This is needed because it's + // a top level class, otherwise the FPRAssert is wrongly trigerred. + if (![self isClassInstrumentable:[NSURLConnection class]]) { + return; + } + FPRClassInstrumentor *instrumentor = [[FPRClassInstrumentor alloc] initWithClass:[NSURLConnection class]]; diff --git a/FirebasePerformance/Sources/Instrumentation/UIKit/FPRUIViewControllerInstrument.m b/FirebasePerformance/Sources/Instrumentation/UIKit/FPRUIViewControllerInstrument.m index 1c2226148ca..e6f7f57c559 100644 --- a/FirebasePerformance/Sources/Instrumentation/UIKit/FPRUIViewControllerInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/UIKit/FPRUIViewControllerInstrument.m @@ -97,6 +97,12 @@ void InstrumentViewDidDisappear(FPRUIViewControllerInstrument *instrument, - (void)registerInstrumentors { dispatch_sync(GetInstrumentationQueue(), ^{ + // Check if it's in the denylist. This is needed because it's + // a top level class, otherwise the FPRAssert is wrongly trigerred. + if (![self isClassInstrumentable:[UIViewController class]]) { + return; + } + FPRClassInstrumentor *instrumentor = [[FPRClassInstrumentor alloc] initWithClass:[UIViewController class]];