Avoid crashes caused by multiple DnsAttempts

When DnsUDPAttempt gets a mismatched response, it returns
ERR_DNS_MALFORMED_RESPONSE but keeps the attempt running while another
one starts. DnsTransaction doesn't handle this case very well. This CL
handles the two crashes that the fuzzer has found and adds a regression
test that catches both of them. There's a bug filed
(https://crbug.com/779589) for a more complete fix.

Bug: 768150, 774846
Change-Id: I0630d73a2e0d1cc179e7fe2139c030d58d25d90f
Reviewed-on: https://chromium-review.googlesource.com/743810Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512864}

net/dns/dns_transaction.cc

@@ -670,8 +670,13 @@ class DnsTransactionImpl : public DnsTransaction,
   }
 
   void DoCallback(AttemptResult result) {
-    DCHECK(!callback_.is_null());
     DCHECK_NE(ERR_IO_PENDING, result.rv);
+
+    // TODO(mgersh): consider changing back to a DCHECK once
+    // https://crbug.com/779589 is fixed.
+    if (callback_.is_null())
+      return;
+
     const DnsResponse* response = result.attempt ?
         result.attempt->GetResponse() : NULL;
     CHECK(result.rv != OK || response != NULL);
@@ -883,8 +888,12 @@ class DnsTransactionImpl : public DnsTransaction,
           session_->RecordServerSuccess(result.attempt->server_index());
           net_log_.EndEventWithNetErrorCode(
               NetLogEventType::DNS_TRANSACTION_QUERY, result.rv);
-          // Try next suffix.
-          qnames_.pop_front();
+          // Try next suffix. Check that qnames_ isn't already empty first,
+          // which can happen when there are two attempts running at once.
+          // TODO(mgersh): remove this workaround for https://crbug.com/774846
+          // when https://crbug.com/779589 is fixed.
+          if (!qnames_.empty())
+            qnames_.pop_front();
           if (qnames_.empty()) {
             return AttemptResult(ERR_NAME_NOT_RESOLVED, NULL);
           } else {

net/dns/dns_transaction_unittest.cc

@@ -643,6 +643,26 @@ TEST_F(DnsTransactionTest, MismatchedResponseFail) {
   EXPECT_TRUE(helper0.RunUntilDone(transaction_factory_.get()));
 }
 
+TEST_F(DnsTransactionTest, MismatchedResponseNxdomain) {
+  config_.attempts = 2;
+  config_.timeout = TestTimeouts::tiny_timeout();
+  ConfigureFactory();
+
+  // First attempt receives mismatched response followed by valid NXDOMAIN
+  // response.
+  // Second attempt receives valid NXDOMAIN response.
+  std::unique_ptr<DnsSocketData> data(
+      new DnsSocketData(0 /* id */, kT0HostName, kT0Qtype, SYNCHRONOUS, false));
+  data->AddResponseData(kT1ResponseDatagram, arraysize(kT1ResponseDatagram),
+                        SYNCHRONOUS);
+  data->AddRcode(dns_protocol::kRcodeNXDOMAIN, ASYNC);
+  AddSocketData(std::move(data));
+  AddSyncQueryAndRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeNXDOMAIN);
+
+  TransactionHelper helper0(kT0HostName, kT0Qtype, ERR_NAME_NOT_RESOLVED);
+  EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
+}
+
 TEST_F(DnsTransactionTest, ServerFail) {
   AddAsyncQueryAndRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeSERVFAIL);