Bug 208029 – REGRESSION (r255533) Null Deref of _sessionWrapper under [WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Null Derek of _sessionWrapper under [WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]:
Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000030)
[  0] 0x00007fff465ce9a0 WebKit`-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:] [inlined] WTF::HashMap<unsigned long long, WebKit::DownloadID, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::DownloadID> >::get(unsigned long long const&) const at HashMap.h:436:12

     0x00007fff465ce98e:     xorl %ebx, %ebx
     0x00007fff465ce990:     movq 0x651289(%rip), %rsi ; ""
     0x00007fff465ce997:     movq %r14, %rdi
     0x00007fff465ce99a:    callq *0x5f90d0(%rip)      ; (void *)0x0000000000000000
 ->  0x00007fff465ce9a0:     movq 0x30(%rbx), %rdi
     0x00007fff465ce9a4:     movl 0x3c(%rbx), %esi
     0x00007fff465ce9a7:     movq %rax, %rdx
     0x00007fff465ce9aa:    callq 0x10f622             ; WTF::HashMap<unsigned long long, WebKit::DownloadID, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::DownloadID> >::get<WTF::IdentityHashTranslator<WTF::HashMap<unsigned long long, WebKit::DownloadID, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::DownloadID> >::KeyValuePairTraits, WTF::IntHash<unsigned long long> >, unsigned long long> at HashMap.h:320
     0x00007fff465ce9af:    testq %rax, %rax

[  0] 0x00007fff465ce9a0 WebKit`-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:] + 109 at NetworkSessionCocoa.mm:617
       613 	    auto* networkDataTask = [self existingTask:task];
       614 	    auto* sessionCocoa = networkDataTask ? static_cast<NetworkSessionCocoa*>(networkDataTask->networkSession()) : nullptr;
       615 	    if (!networkDataTask) {
       616 	        ASSERT(!sessionCocoa);
    -> 617 	        auto downloadID = _sessionWrapper->downloadMap.get(task.taskIdentifier);
       618 	        auto download = downloadID.downloadID() ? _session->networkProcess().downloadManager().download(downloadID) : nil;
       619 	        sessionCocoa = download ? static_cast<NetworkSessionCocoa*>(_session->networkProcess().networkSession(download->sessionID())) : nil;
       620 	    }
       621 	    if (!sessionCocoa || [task state] == NSURLSessionTaskStateCanceling) {
    
[  1] 0x00007fff339ce5c4 CFNetwork`__68-[NSURLSession delegate_task:didReceiveChallenge:completionHandler:]_block_invoke + 138 at Session.mm:598:3
       594 	{
       595 		id<NSURLSessionTaskDelegate> d = (id<NSURLSessionTaskDelegate>) _delegate_ivar;
       596 		[self addDelegateBlock:^{
       597 			[task._metrics delegateBegin:@selector(URLSession:task:didReceiveChallenge:completionHandler:)];
    -> 598 			[d URLSession:self task:task didReceiveChallenge:challenge completionHandler:^(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential) {
       599 				[task._metrics delegateEnd:@selector(URLSession:task:didReceiveChallenge:completionHandler:)];
       600 				completionHandler(disposition, credential);
       601 			}];
       602 		}];
    
[  2] 0x00007fff3787cc54 Foundation`__NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__ + 6 at NSOperation.m:1541:5
       1537	}
       1538	
       1539	static void __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__(void (^block)(void)) __attribute__((noinline));
       1540	static void __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__(void (^block)(void)) {
    -> 1541	    block();
       1542	    __asm __volatile__(""); // thwart tail-call optimization
       1543	}
       1544	
       1545	- (void)main {

[Chris Dumez]

<rdar://problem/59404381>

[Chris Dumez]

Created attachment 391329 [details]
Patch

[David Quesada]

Comment on attachment 391329 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391329&action=review

Makes sense to me. (But I'm not a reviewer)

Thanks for fixing this!

> Source/WebKit/ChangeLog:3
> +        REGRESSION (r255533) Null Derek of _sessionWrapper under [WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]

"Null Derek" should be "Null deref"

> Source/WebKit/ChangeLog:9
> +        r255533 started deferencing _sessionWrapper without null check in didReceiveChallenge. All other delegates in this file null check

"deferencing" should be "dereferencing"

[Chris Dumez]

Created attachment 391330 [details]
Patch

[Chris Dumez]

Comment on attachment 391330 [details]
Patch

Clearing flags on attachment: 391330

Committed r257158: <https://trac.webkit.org/changeset/257158>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.