Added browser test for TemplateUrlService protected prefs

chrome/browser/prefs/tracked/pref_hash_browsertest.cc

Index: chrome/browser/prefs/tracked/pref_hash_browsertest.cc
diff --git a/chrome/browser/prefs/tracked/pref_hash_browsertest.cc b/chrome/browser/prefs/tracked/pref_hash_browsertest.cc
index f77616a6b84ab5faa574b096939d1e39677681e3..99c26d8d9faeedddbc202691c8aeb3b0160a3cc8 100644
--- a/chrome/browser/prefs/tracked/pref_hash_browsertest.cc
+++ b/chrome/browser/prefs/tracked/pref_hash_browsertest.cc
@@ -10,12 +10,14 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
+#include "base/json/json_reader.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
@@ -30,6 +32,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/search_engines/default_search_manager.h"
+#include "components/search_engines/template_url_data.h"
 #include "components/user_prefs/tracked/tracked_preference_histogram_names.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/extension.h"
@@ -1171,3 +1174,121 @@ class PrefHashBrowserTestRegistryValidationFailure
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestRegistryValidationFailure,
                        RegistryValidationFailure);
 #endif
+
+static constexpr char default_search_provider_data[] =

[gab, 2016/12/07 16:18:44]

No need for "static" keyword at file-scope

[Peter Kasting, 2016/12/07 18:40:18]

Better yet, make these function-scope, since they're only used within a single
function.

[Alexander Yashkin, 2016/12/08 04:53:57]

Done

+    R"({

[Peter Kasting, 2016/12/07 18:40:18]

Nit: I suggest formatting these like:

static constexpr char default_search_provider_data[] = R"(
{
  "default_search_provider_data" : {
    "template_url_data" : {
      "keyword" : "unknown",
      ...
    }
  }
})";
...
  "search_provider_overrides" : [
    {
      "keyword" : "unknown"
      ...
    }, {
      ...
    }
  ]
...

* Starting at the beginning of the line helps to reduce excessive indenting and
thus >80-column lines
* Placing the opening brace at the end of the previous line matches how you do
list formatting for search_provider_overrides and in general shortens things
without harming readability; this also fixes an erroneous one-space brace
indentation
* Modified indenting/wrapping of list items eliminates successive symbols at the
same indentation level (which is error-prone)

[Peter Kasting, 2016/12/07 18:41:34]

(Note, read the above comment on the review tool, as email will strip the
leading whitespace)

[Alexander Yashkin, 2016/12/08 04:53:57]

Tried to follow your guide, yet not sure if I succeed. :)
Maybe add it to chromium C++ code style? I saw a lot of embedded json in
chromium test code.

+          "default_search_provider_data" :
+           {
+            "template_url_data" :
+            {
+              "keyword" : "malwarekeyword",

[gab, 2016/12/07 16:18:44]

Let's not use "malware" in our codebase, "bad" or "unknown" will work just as
well.

[Alexander Yashkin, 2016/12/08 04:53:57]

replaced with "bad"

+              "short_name" : "malwarename",
+              "url" : "http://malware_default_engine/search?q=dirty_user_query"
+            }
+          }
+        })";
+static constexpr char search_provider_overrides[] =
+    R"({
+          "search_provider_overrides" : [
+          {
+            "keyword" : "malwarekeyword",
+            "name" : "malwarename",
+            "search_url" : "http://malware_default_engine/search?q=dirty_user_query",
+            "encoding" : "utf-8",
+            "id" : 1
+          },
+          {
+            "keyword" : "malwarekeyword2",
+            "name" : "malwarename2",
+            "search_url" : "http://malware_default_engine2/search?q=dirty_user_query",
+            "encoding" : "utf-8",
+            "id" : 2
+          }
+          ]
+        })";
+static constexpr char default_search_provider[] =
+    R"({
+          "default_search_provider" :
+          {
+            "keyword" : "malwarekeyword",
+            "name" : "malwarename",
+            "search_url" : "http://malware_default_engine/search?q=dirty_user_query"
+          }
+        })";
+
+// Verifies that all preferences related to choice of default search engine are
+// protected.
+class TestDefaultSearchProtected : public PrefHashBrowserTestBase {

[gab, 2016/12/07 16:18:44]

All other fixtures in this file are prefixed with PrefHashBrowserTest, let's do
this for this one too (allows --gtest_filter=*PrefHashBrowserTest*)

[Alexander Yashkin, 2016/12/08 04:53:57]

Done

+ public:
+  void SetupPreferences() override {
+    // Set user selected default search engine.
+    DefaultSearchManager default_search_manager(
+        profile()->GetPrefs(), DefaultSearchManager::ObserverCallback());
+    DefaultSearchManager::Source dse_source =
+        static_cast<DefaultSearchManager::Source>(-1);
+
+    TemplateURLData user_dse;
+    user_dse.SetKeyword(base::UTF8ToUTF16("userkeyword"));
+    user_dse.SetShortName(base::UTF8ToUTF16("username"));
+    user_dse.SetURL("http://user_default_engine/search?q=good_user_query");
+    default_search_manager.SetUserSelectedDefaultSearchEngine(user_dse);
+
+    const TemplateURLData* current_dse =
+        default_search_manager.GetDefaultSearchEngine(&dse_source);
+    EXPECT_EQ(DefaultSearchManager::FROM_USER, dse_source);
+    EXPECT_EQ(current_dse->keyword(), base::UTF8ToUTF16("userkeyword"));
+    EXPECT_EQ(current_dse->short_name(), base::UTF8ToUTF16("username"));
+    EXPECT_EQ(current_dse->url(),
+              "http://user_default_engine/search?q=good_user_query");
+  }
+
+  void AttackPreferencesOnDisk(
+      base::DictionaryValue* unprotected_preferences,
+      base::DictionaryValue* protected_preferences) override {
+    // Try to override default search in all three of available preferences.
+    auto attack1 = base::DictionaryValue::From(
+        base::JSONReader::Read(default_search_provider_data));
+    auto attack2 = base::DictionaryValue::From(
+        base::JSONReader::Read(search_provider_overrides));
+    auto attack3 = base::DictionaryValue::From(
+        base::JSONReader::Read(default_search_provider));
+    unprotected_preferences->MergeDictionary(attack1.get());
+    unprotected_preferences->MergeDictionary(attack2.get());
+    unprotected_preferences->MergeDictionary(attack3.get());
+    if (protected_preferences) {
+      // Override here, too.
+      protected_preferences->MergeDictionary(attack1.get());
+      protected_preferences->MergeDictionary(attack2.get());
+      protected_preferences->MergeDictionary(attack3.get());
+    }
+  }
+
+  void VerifyReactionToPrefAttack() override {
+    // Check that no malware written engine is active.
+    DefaultSearchManager default_search_manager(
+        profile()->GetPrefs(), DefaultSearchManager::ObserverCallback());
+    DefaultSearchManager::Source dse_source =
+        static_cast<DefaultSearchManager::Source>(-1);
+
+    const TemplateURLData* current_dse =
+        default_search_manager.GetDefaultSearchEngine(&dse_source);
+
+    if (protection_level_ < PROTECTION_ENABLED_DSE) {
+      // Attack is successfull.

[Peter Kasting, 2016/12/07 18:40:18]

Nit: Only one l

[Alexander Yashkin, 2016/12/08 04:53:57]

Successfully corrected :)

+      EXPECT_EQ(DefaultSearchManager::FROM_USER, dse_source);
+      EXPECT_EQ(current_dse->keyword(), base::UTF8ToUTF16("malwarekeyword"));
+      EXPECT_EQ(current_dse->short_name(), base::UTF8ToUTF16("malwarename"));
+      EXPECT_EQ(current_dse->url(),
+                "http://malware_default_engine/search?q=dirty_user_query");
+    } else {
+      // Attack fails.
+      EXPECT_EQ(DefaultSearchManager::FROM_FALLBACK, dse_source);
+      EXPECT_NE(current_dse->keyword(), base::UTF8ToUTF16("malwarekeyword"));
+      EXPECT_NE(current_dse->short_name(), base::UTF8ToUTF16("malwarename"));
+      EXPECT_NE(current_dse->url(),
+                "http://malware_default_engine/search?q=dirty_user_query");
+    }
+  }
+};
+
+PREF_HASH_BROWSER_TEST(TestDefaultSearchProtected, DefaultSearch);