Safely copy MIMEType to prevent use after free (Issue #14734) #15313
Conversation
Copied response.MIMEType before assigning to responseContentType to avoid EXC_BAD_ACCESS caused by early deallocation under certain network or SDK conditions.
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Thank you for this contribution. The pull request description provides an excellent and detailed analysis of the use-after-free bug, which is very helpful. The fix itself is correct and effectively prevents the crash by copying the MIMEType. I've added one minor suggestion to simplify the implementation slightly by removing a redundant local variable. Overall, this is a great fix.
| // Safely copy MIMEType to prevent use after free | ||
| NSString *mime = [response.MIMEType copy]; | ||
| self.responseContentType = (mime.length ? mime : nil); |
There was a problem hiding this comment.
Thanks, @JesusRojass, for investigating! I'm unsure if this approach addresses the issue. The crash happens in the call to MIMEType, so the subsequent copy won't run (3 CFNetwork 0x32c9c URLResponse::getMIMEType() + 60).
There was a problem hiding this comment.
I still have doubts that this is fixes the crash in the linked issue, but I think this is a great improvement that makes this logic more resilient and explicit, so we can merge as-is and continue to monitor the original issue for any changes.
There was a problem hiding this comment.
I have another more defensive idea to address the crash because I never thought of it that way
just waiting on my local unit tests to run before commiting
|
@visumickey tag in merged pr |
3 participants
Copied response.MIMEType before assigning to responseContentType to avoid EXC_BAD_ACCESS caused by early deallocation under certain network or SDK conditions.
Discussion
Clients state that they where experiencing a crash in FPRNetworkTrace on devices running iOS and iPadOS 17.5.1 and up, its not a 100% reproducible crash but it seems to happen under certain network conditions
After I began to read the stack trace and logs I found out that the root cause is a use after bug that is triggered by accessing response.MIMEType from a possibly deallocated NSURLResponse object inside the didCompleteRequestWithResponse method located inside of FPRNetworkTrace.m
Where the crash happens on the user provided stack trace (EXC_BAD_ACCESS (SIGSEGV) type crash):
0 libsystem_malloc.dylib __CFStringDeallocate 1 CoreFoundation _CFRelease 2 CFNetwork URLResponse::getMIMEType() 3 FPRNetworkTrace didCompleteRequestWithResponse:error:The MIME type string returned by NSURLResponse.MIMEType is a non retained CoreFoundation owned pointer that may be deallocated if the NSURLResponse instance is no longer strongly referenced or becomes invalid if the delegate method is invoked on a separate queue after the response’s lifecycle ends, without retaining a response, this opens a race condition where response may be freed before MIMEType is accessed.
Possible repro scenarios (Hard to reproduce!!!)
Proposed Fix
My proposed fix consists of modifying the assignment of response.MIMEType in -didCompleteRequestWithResponse:error: to defensively copy the value, this will address the crash users had and adds an extra layer of safety when this value is being accessed, and it also doesn’t add or makes a behavioral change to the existing flow also under certain network conditions or when third-party SDKs like AppLovin hook into NSURLSession, the original NSURLResponse instance may be prematurely deallocated or altered, copying the MIME type ensures the string is retained independently on the heap, breaking potential races with early deallocation or thread-unsafe memory access.
Testing
Testing Screenshots
iPhone 14 Pro
iPhone SE 3rd Gen Simulator
API Changes