Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/01d1d1c15c54f84470463190d2da98e5b6755d97 (HostResolverImpl fuzzer fixes).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

mgersh no longer works on Chrome, so unassigning.

This is not a security bug, since it is a problem with the fuzzer itself.

Unable to provide possible suspect using Predator, CL and Code Search.
Could someone please look into the issue.
Thank You.

Note - We don't have access to the test case: https://clusterfuzz.com/testcase?key=5370943732711424

Eric:  This looks like a real regression, calling into a deleted DnsTransactionImpl.  I'm reverting the original CL until we have this worked out.

For the record, I'm not positive this failure was caused by your CL, the timing, though, is very suspicious.

A cursed CL.  We already rolled it back and fixed it for a UAF once before.  I'll look into it and hopefully the 3rd time will be the charm.

Yea, it happens.  It's often as much a sign of design issues in the area the CL touches, than of issues with the CL itself.  I think our DNS code is badly in need of cleanup.

So the bug was created shortly after landing my CL, but the clusterfuzz report says "Created:	Fri, Jun 22, 2018, 9:36 PM".  That's before I landed even the original version.  Is that date a reliable indication of when the regression started or is that just some indication of when clusterfuzz started looking for issues?

And are any of these dates a reliable indication of when the regression started at all, or are they just purely when some random fuzzing first happened to hit a bug that could potentially be much older?

Ahh...Yea, I think it probably is.  Didn't catch that because I couldn't access it.  It apparently requires a chromium.org account, and doesn't support multi-login.  :(

I've cancelled the revert.  If this isn't due to your CL, I'm happy to take this myself (Though feel free to take it, if you want)

I think I've got it figured out.  It looks like https://chromium-review.googlesource.com/c/chromium/src/+/817681 wasn't as complete as thought at fixing its issue.  If that's the case, this bug is many months old.

The memory being accessed are DnsAttempt objects, ownership of which are maintained in the attempts_ Vector.  We access them through a raw pointer on processing completed attempts, and some of that access occurs after a PostTask.  But in MakeTcpAttempt(), there's code to delete non-completed attempts (the filter to only delete if non-completed is what that CL above added).  Problem is that completed attempts are detected using using is_completed(), but that method seems to ignore retriable errors (eg ERR_DNS_TIMED_OUT) that have exhausted retries.  In such cases, we're done processing it, so it goes on to the result processing code, but it's still eligible for deletion during the PostTask.

Best fix is probably to have an explicit done_ flag we can set when we're no longer going to make any retries.  Also, instead of using a raw pointer, switch to a weakptr that we can at least DCHECK to help find any additional errors and let us safely crash.

Great!  Thanks for the quick investigation, Eric!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f7f0615b9d6eeecafd4d0ff0753f65642cc1432

commit 8f7f0615b9d6eeecafd4d0ff0753f65642cc1432
Author: Eric Orth <ericorth@chromium.org>
Date: Fri Jul 13 21:58:35 2018

Fix UAF in DnsTransaction

Starting a TCP DNS attempt cancels and deletes any previous running
attempts.  Fix the filtering to skip retriable errors that will not
retry because we have exhausted available attempts.  Otherwise, if an
attempt has already completed synchronously and posted to handle the
result, it could try to handle the result of a deleted attempt.

Also replace the raw Attempt pointer in AttemptResult with a WeakPtr, so
any future similar bugs will safely DCHECK and crash instead of UAF.

Bug:  860292 
Change-Id: I4a012a078b9b35f87b95a9592f4e43573c6a0708
Reviewed-on: https://chromium-review.googlesource.com/1134028
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575085}
[modify] https://crrev.com/8f7f0615b9d6eeecafd4d0ff0753f65642cc1432/net/dns/dns_transaction.cc
[modify] https://crrev.com/8f7f0615b9d6eeecafd4d0ff0753f65642cc1432/net/dns/dns_transaction_unittest.cc

ClusterFuzz has detected this issue as fixed in range 575063:575086.

Detailed report: https://clusterfuzz.com/testcase?key=5370943732711424

Fuzzer: afl_net_host_resolver_impl_fuzzer
Job Type: afl_chrome_asan
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60600000c020
Crash State:
  net::DnsTransactionImpl::DoCallback
  base::debug::TaskAnnotator::RunTask
  base::MessageLoop::RunTask
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=afl_chrome_asan&range=503738:503808
Fixed: https://clusterfuzz.com/revisions?job=afl_chrome_asan&range=575063:575086

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5370943732711424

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5370943732711424 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.