Track permissions granted to extensions in prefs

chrome/common/extensions/extension.cc

 // Copyright (c) 2010 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 "chrome/common/extensions/extension.h"
 
 #include <algorithm>
 
 #include "app/l10n_util.h"
 #include "base/base64.h"
@@ skipping 1061 matching lines @@
   output->append(" ");
   output->append(is_public ? kPublic : kPrivate);
   output->append(" ");
   output->append(kKeyInfoEndMarker);
   output->append("\n");
 
   return true;
 }
 
 // static
-// TODO(aa): A problem with this code is that we silently allow upgrades to
-// extensions that require less permissions than the current version, but then
-// we don't silently allow them to go back. In order to fix this, we would need
-// to remember the max set of permissions we ever granted a single extension.
-bool Extension::IsPrivilegeIncrease(const Extension* old_extension,
+bool Extension::IsPrivilegeIncrease(const std::set<std::string>& granted_apis,
+                                    const ExtensionExtent& granted_extent,
+                                    const Extension* old_extension,
                                     const Extension* new_extension) {
-  // If the old extension had native code access, we don't need to go any
-  // further. Things can't get any worse.
-  if (old_extension->plugins().size() > 0)
-    return false;
+  // COMPONENT extensions should never go through this code path.
+  DCHECK(new_extension->location() != Extension::COMPONENT);

[Aaron Boodman, 2010/11/22 07:57:53]

Two things:

1) DCHECK is for wusses, and causes lots of problems. It is compiled out of
release builds, so execution continues past broken assumptions and we finally
crash somewhere else, long past the actual point of failure. When you find
yourself using DCHECK consider whether a) this really cannot happen ever unless
a programmer has made a mistake (in which case use CHECK), or b) this could
happen if some external system was misbehaving or not meeting our expectations
(in which case handle the error gracefully).

2) In this case, I think the [D]CHECK is not buying anything. This is a nice
generic function that we might want to use in other scenarios in the future. In
the current configuration we do not call it with component extensions, but there
is no reason to lock it down.

[jstritar, 2010/11/22 23:01:08]

Thanks for the explanation... I went a little crazy with the DHCECKs :). I
removed this one, and most of the others.

 
-  // Otherwise, if the new extension has a plugin, it's a privilege increase.
-  if (new_extension->plugins().size() > 0)
-    return true;
+  if (old_extension) {
+    // If the old extension had native code access, we don't need to go any
+    // further. Things can't get any worse.
+    if (old_extension->plugins().size() > 0)
+      return false;
 
-  // If we are increasing the set of hosts we have access to (not
-  // counting scheme differences), it's a privilege increase.
-  if (!old_extension->HasEffectiveAccessToAllHosts()) {
+    // Otherwise, if the new extension has a plugin, it's a privilege increase.
+    if (new_extension->plugins().size() > 0)
+      return true;
+  }
+
+  // If the extension hadn't been granted access to all hosts in the past, then
+  // see if the extension requires more host permissions.
+  if (!HasEffectiveAccessToAllHosts(granted_extent, granted_apis)) {
     if (new_extension->HasEffectiveAccessToAllHosts())
       return true;
 
-    // TODO(erikkay) This will trip when you add a new distinct hostname,
-    // but we should unique based on RCD as well.  crbug.com/57042
-    std::vector<std::string> old_hosts = old_extension->GetDistinctHosts();
-    std::vector<std::string> new_hosts = new_extension->GetDistinctHosts();
+    const ExtensionExtent new_extent =
+        new_extension->GetEffectiveHostPermissions();
+    std::vector<std::string> new_hosts =
+        GetDistinctHosts(new_extent.patterns());
+    std::vector<std::string> old_hosts =
+        GetDistinctHosts(granted_extent.patterns());
+
     std::set<std::string> old_hosts_set(old_hosts.begin(), old_hosts.end());
     std::set<std::string> new_hosts_set(new_hosts.begin(), new_hosts.end());
-    std::set<std::string> new_only;
+    std::set<std::string> new_hosts_only;
+
     std::set_difference(new_hosts_set.begin(), new_hosts_set.end(),
-                      old_hosts_set.begin(), old_hosts_set.end(),
-                      std::inserter(new_only, new_only.end()));
-    if (new_only.size())
+                        old_hosts_set.begin(), old_hosts_set.end(),
+                        std::inserter(new_hosts_only, new_hosts_only.end()));
+
+    if (new_hosts_only.size())
       return true;
   }
 
-  std::set<string16> old_messages =
-      old_extension->GetSimplePermissionMessages();
-  std::set<string16> new_messages =
-      new_extension->GetSimplePermissionMessages();
-  std::set<string16> new_only;
-  std::set_difference(new_messages.begin(), new_messages.end(),
-                      old_messages.begin(), old_messages.end(),
-                      std::inserter(new_only, new_only.end()));
+  std::set<std::string> new_apis = new_extension->api_permissions();
+  std::set<std::string> new_apis_only;
+  std::set_difference(new_apis.begin(), new_apis.end(),
+                      granted_apis.begin(), granted_apis.end(),
+                      std::inserter(new_apis_only, new_apis_only.end()));
 
-  // If there are any new permission messages, then it's an increase.
-  if (!new_only.empty())
+  // Ignore API permissions that don't require user approval when deciding if
+  // an extension has increased its privileges.
+  size_t new_api_count = 0;
+  for (std::set<std::string>::iterator i = new_apis_only.begin();
+       i != new_apis_only.end(); ++i) {
+    if (GetPermissionMessageId(*i))
+      new_api_count++;
+  }
+
+  if (new_api_count)
     return true;
 
   return false;
 }
 
 // static
 void Extension::DecodeIcon(const Extension* extension,
                            Icons icon_size,
                            scoped_ptr<SkBitmap>* result) {
   FilePath icon_path = extension->GetIconResource(
@@ skipping 561 matching lines @@
           continue;
         }
       } else {
         // Hosted apps only get access to a subset of the valid permissions.
         if (IsHostedAppPermission(permission_str)) {
           api_permissions_.insert(permission_str);
           continue;
         }
       }
 
-      // Otherwise, it's a host pattern permission.
+      // Check if it's a host pattern permission.
       URLPattern pattern = URLPattern(CanExecuteScriptEverywhere() ?
           URLPattern::SCHEME_ALL :
           (UserScript::kValidUserScriptSchemes |
               URLPattern::SCHEME_CHROMEUI) & ~URLPattern::SCHEME_FILE);
 
-      if (URLPattern::PARSE_SUCCESS != pattern.Parse(permission_str)) {
-        *error = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPermission, base::IntToString(i));
-        return false;
+      if (URLPattern::PARSE_SUCCESS == pattern.Parse(permission_str)) {
+        if (!CanSpecifyHostPermission(pattern)) {
+          *error = ExtensionErrorUtils::FormatErrorMessage(
+              errors::kInvalidPermissionScheme, base::IntToString(i));
+          return false;
+        }
+
+        // The path component is not used for host permissions, so we force it
+        // to match all paths.
+        pattern.set_path("/*");
+
+        host_permissions_.push_back(pattern);
       }
 
-      if (!CanSpecifyHostPermission(pattern)) {
-        *error = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPermissionScheme, base::IntToString(i));
-        return false;
-      }
-
-      // The path component is not used for host permissions, so we force it to
-      // match all paths.
-      pattern.set_path("/*");
-
-      host_permissions_.push_back(pattern);
+      // If it's not a host permission, then it's probably an unknown API
+      // permission. Do not throw an error so extensions can retain
+      // backwards compatability (http://crbug.com/42742).
+      // TODO(jstritar): We could add better validation of API permissions here
+      //   if we'd like to improve error messages.
     }
   }
 
   if (source.HasKey(keys::kDefaultLocale)) {
     if (!source.GetString(keys::kDefaultLocale,
                           &default_locale_) ||
         default_locale_.empty()) {
       *error = errors::kInvalidDefaultLocale;
       return false;
     }
@@ skipping 373 matching lines @@
   }
 
   if (error) {
     *error = ExtensionErrorUtils::FormatErrorMessage(errors::kCannotAccessPage,
                                                      page_url.spec());
   }
 
   return false;
 }
 
-bool Extension::HasEffectiveAccessToAllHosts() const {
+// static
+bool Extension::HasEffectiveAccessToAllHosts(
+    const ExtensionExtent& effective_host_permissions,
+    const std::set<std::string>& api_permissions) {
   // Some APIs effectively grant access to every site.  New ones should be
   // added here.  (I'm looking at you, network API)
-  if (HasApiPermission(kProxyPermission))
+  if (HasApiPermission(api_permissions, kProxyPermission))
     return true;
 
-  for (URLPatternList::const_iterator host = host_permissions().begin();
-       host != host_permissions().end(); ++host) {
+  const URLPatternList patterns = effective_host_permissions.patterns();
+  for (URLPatternList::const_iterator host = patterns.begin();
+       host != patterns.end(); ++host) {
     if (host->match_subdomains() && host->host().empty())
       return true;
   }
 
-  for (UserScriptList::const_iterator content_script =
-       content_scripts().begin();
-       content_script != content_scripts().end(); ++content_script) {
-    UserScript::PatternList::const_iterator pattern =
-        content_script->url_patterns().begin();
-    for (; pattern != content_script->url_patterns().end(); ++pattern) {
-      if (pattern->match_subdomains() && pattern->host().empty())
-        return true;
-    }
-  }
-
   return false;
 }
 
+bool Extension::HasEffectiveAccessToAllHosts() const {
+  return HasEffectiveAccessToAllHosts(GetEffectiveHostPermissions(),
+                                      api_permissions());
+}
+
 bool Extension::IsAPIPermission(const std::string& str) const {
   for (size_t i = 0; i < Extension::kNumPermissions; ++i) {
     if (str == Extension::kPermissions[i].name) {
       // Only allow the experimental API permission if the command line
       // flag is present, or if the extension is a component of Chrome.
       if (str == Extension::kExperimentalPermission) {
         if (CommandLine::ForCurrentProcess()->HasSwitch(
                 switches::kEnableExperimentalExtensionApis)) {
           return true;
         } else if (location() == Extension::COMPONENT) {
@@ skipping 48 matching lines @@
 UninstalledExtensionInfo::UninstalledExtensionInfo(
     const Extension& extension)
     : extension_id(extension.id()),
       extension_api_permissions(extension.api_permissions()),
       is_theme(extension.is_theme()),
       is_app(extension.is_app()),
       converted_from_user_script(extension.converted_from_user_script()),
       update_url(extension.update_url()) {}
 
 UninstalledExtensionInfo::~UninstalledExtensionInfo() {}