Better crash stacktraces for ExtensionFunction bad messages.

extensions/browser/extension_function.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/extension_function.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/synchronization/lock.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "extensions/browser/bad_message.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_message_filter.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension_api.h"
 #include "extensions/common/extension_messages.h"
 
 using content::BrowserThread;
 using content::RenderViewHost;
 using content::WebContents;
@@ skipping 38 matching lines @@
                                   histogram_value);
     } else {
       UMA_HISTOGRAM_SPARSE_SLOWLY("Extensions.Functions.FailedTime.Over10ms",
                                   histogram_value);
     }
     UMA_HISTOGRAM_TIMES("Extensions.Functions.FailedTotalExecutionTime",
                         elapsed_time);
   }
 }
 
+void LogBadMessage(extensions::functions::HistogramValue histogram_value) {
+  content::RecordAction(base::UserMetricsAction("BadMessageTerminate_EFD"));
+  // Track the specific function's |histogram_value|, as this may indicate a
+  // bug in that API's implementation.
+  UMA_HISTOGRAM_ENUMERATION("Extensions.BadMessageFunctionName",
+                            histogram_value,
+                            extensions::functions::ENUM_BOUNDARY);
+}
+
+template <class T>
+void ReceivedBadMessage(T* bad_message_sender,
+                        extensions::bad_message::BadMessageReason reason,
+                        extensions::functions::HistogramValue histogram_value) {
+  LogBadMessage(histogram_value);
+  // The renderer has done validation before sending extension api requests.
+  // Therefore, we should never receive a request that is invalid in a way
+  // that JSON validation in the renderer should have caught. It could be an
+  // attacker trying to exploit the browser, so we crash the renderer instead.
+  extensions::bad_message::ReceivedBadMessage(bad_message_sender, reason);
+}
+
 class ArgumentListResponseValue
     : public ExtensionFunction::ResponseValueObject {
  public:
   ArgumentListResponseValue(ExtensionFunction* function,
                             std::unique_ptr<base::ListValue> result) {
     SetFunctionResults(function, std::move(result));
     // It would be nice to DCHECK(error.empty()) but some legacy extension
     // function implementations... I'm looking at chrome.input.ime... do this
     // for some reason.
   }
@@ skipping 26 matching lines @@
   }
 
   ~ErrorResponseValue() override {}
 
   bool Apply() override { return false; }
 };
 
 class BadMessageResponseValue : public ExtensionFunction::ResponseValueObject {
  public:
   explicit BadMessageResponseValue(ExtensionFunction* function) {
-    function->set_bad_message(true);
+    function->SetBadMessage();
     NOTREACHED() << function->name() << ": bad message";
   }
 
   ~BadMessageResponseValue() override {}
 
   bool Apply() override { return false; }
 };
 
 class RespondNowAction : public ExtensionFunction::ResponseActionObject {
  public:
@@ skipping 176 matching lines @@
 }
 
 const base::ListValue* ExtensionFunction::GetResultList() const {
   return results_.get();
 }
 
 const std::string& ExtensionFunction::GetError() const {
   return error_;
 }
 
+void ExtensionFunction::SetBadMessage() {

[Devlin, 2017/01/27 17:38:58]

Does this compile?  ExtensionFunction::SetBadMessage() is pure virtual in the
header.

[lazyboy, 2017/01/27 18:17:24]

Ah, I've put a mental note to fix it after noticing that this surprisingly
compiled locally: both chrome & browser_tests target were fine. But forgot.

Needs a separate look into this issue. Will do.

Done.

+  bad_message_ = true;
+}
+
 bool ExtensionFunction::user_gesture() const {
   return user_gesture_ || UserGestureForTests::GetInstance()->HaveGesture();
 }
 
 ExtensionFunction::ResponseValue ExtensionFunction::NoArguments() {
   return ResponseValue(
       new ArgumentListResponseValue(this, base::MakeUnique<base::ListValue>()));
 }
 
 ExtensionFunction::ResponseValue ExtensionFunction::OneArgument(
@@ skipping 169 matching lines @@
   // here and use weakptrs for the tasks, then have it owned by something that
   // will be destroyed naturally in the course of shut down.
   if (extensions::ExtensionsBrowserClient::Get()->IsShuttingDown()) {
     *error = "The browser is shutting down.";
     return false;
   }
 
   return true;
 }
 
+void UIThreadExtensionFunction::SetBadMessage() {
+  ExtensionFunction::SetBadMessage();
+
+  if (render_frame_host()) {
+    ReceivedBadMessage(render_frame_host()->GetProcess(),
+                       is_from_service_worker()
+                           ? extensions::bad_message::EFD_BAD_MESSAGE_WORKER
+                           : extensions::bad_message::EFD_BAD_MESSAGE,
+                       histogram_value());
+  }
+}
+
 bool UIThreadExtensionFunction::OnMessageReceived(const IPC::Message& message) {
   return false;
 }
 
 void UIThreadExtensionFunction::Destruct() const {
   BrowserThread::DeleteOnUIThread::Destruct(this);
 }
 
 void UIThreadExtensionFunction::SetRenderFrameHost(
     content::RenderFrameHost* render_frame_host) {
@@ skipping 49 matching lines @@
 }
 
 IOThreadExtensionFunction::~IOThreadExtensionFunction() {
 }
 
 IOThreadExtensionFunction*
 IOThreadExtensionFunction::AsIOThreadExtensionFunction() {
   return this;
 }
 
+void IOThreadExtensionFunction::SetBadMessage() {
+  ExtensionFunction::SetBadMessage();
+  if (ipc_sender_) {
+    ReceivedBadMessage((content::BrowserMessageFilter*)ipc_sender_.get(),

[Devlin, 2017/01/27 17:38:58]

nit: prefer static_cast<> over c-style casts

[lazyboy, 2017/01/27 18:17:24]

Done.

+                       extensions::bad_message::EFD_BAD_MESSAGE,
+                       histogram_value());
+  }
+}
+
 void IOThreadExtensionFunction::Destruct() const {
   BrowserThread::DeleteOnIOThread::Destruct(this);
 }
 
 AsyncExtensionFunction::AsyncExtensionFunction() {
 }
 
 void AsyncExtensionFunction::SetError(const std::string& error) {
   error_ = error;
 }
@@ skipping 39 matching lines @@
 void AsyncExtensionFunction::SendResponse(bool success) {
   ResponseValue response;
   if (success) {
     response = ArgumentList(std::move(results_));
   } else {
     response = results_ ? ErrorWithArguments(std::move(results_), error_)
                         : Error(error_);
   }
   Respond(std::move(response));
 }