Packaged (CRX) extensions shouldn't be able to get 'dev' interfaces in NaCl.

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
===================================================================
--- chrome/renderer/chrome_content_renderer_client.cc	(revision 105554)
+++ chrome/renderer/chrome_content_renderer_client.cc	(working copy)
@@ -417,54 +417,79 @@
           IDR_CLICK_TO_PLAY_PLUGIN_HTML, IDS_PLUGIN_LOAD, true, true);
     }
 
-    // Enforce the Chrome WebStore restriction on the Native Client plugin.
+    // If this is the NaCl plugin, get the manifest URL for the app so we can
+    // determine if it's OK to run.
     if (is_nacl_plugin) {
-      bool allow_nacl = cmd->HasSwitch(switches::kEnableNaCl);
-      if (!allow_nacl) {
-        const char* kNaClPluginMimeType = "application/x-nacl";
-        const char* kNaClPluginManifestAttribute = "nacl";
+      const char* kNaClPluginMimeType = "application/x-nacl";
+      const char* kNaClPluginManifestAttribute = "nacl";
 
-        GURL nexe_url;
-        if (actual_mime_type == kNaClPluginMimeType) {
-          nexe_url = url;  // Normal embedded NaCl plugin.
-        } else {
-          // Content type handling NaCl plugin; the "nacl" param on the
-          // MIME type holds the nexe URL.
-          string16 nacl_attr = ASCIIToUTF16(kNaClPluginManifestAttribute);
-          for (size_t i = 0; i < plugin.mime_types.size(); ++i) {
-            if (plugin.mime_types[i].mime_type == actual_mime_type) {
-              const webkit::WebPluginMimeType& content_type =
-                  plugin.mime_types[i];
-              for (size_t i = 0;
-                  i < content_type.additional_param_names.size(); ++i) {
-                if (content_type.additional_param_names[i] == nacl_attr) {
-                  nexe_url = GURL(content_type.additional_param_values[i]);
-                  break;
-                }
+      GURL manifest_url;
+      if (actual_mime_type == kNaClPluginMimeType) {
+        manifest_url = url;  // Normal embedded NaCl plugin.
+      } else {
+        // Content type handling NaCl plugin; the "nacl" param on the
+        // MIME type holds the nexe URL.
+        string16 nacl_attr = ASCIIToUTF16(kNaClPluginManifestAttribute);
+        for (size_t i = 0; i < plugin.mime_types.size(); ++i) {
+          if (plugin.mime_types[i].mime_type == actual_mime_type) {
+            const webkit::WebPluginMimeType& content_type =
+                plugin.mime_types[i];
+            for (size_t i = 0;
+                i < content_type.additional_param_names.size(); ++i) {
+              if (content_type.additional_param_names[i] == nacl_attr) {
+                manifest_url = GURL(content_type.additional_param_values[i]);
+                break;
               }
-              break;
             }
+            break;
           }
         }
-
-        // Create the NaCl plugin only if the .nexe is part of an extension
-        // that was installed from the Chrome Web Store, or part of a component
-        // extension, or part of an unpacked extension.
-        const Extension* extension =
-            extension_dispatcher_->extensions()->GetByURL(nexe_url);
-        allow_nacl = extension &&
-            (extension->from_webstore() ||
-            extension->location() == Extension::COMPONENT ||
-            extension->location() == Extension::LOAD);
       }
 
-      if (!allow_nacl) {
+      // Determine if the manifest URL is part of an extension.
+      const Extension* extension =
+          extension_dispatcher_->extensions()->GetByURL(manifest_url);
+      // Only component, unpacked, and Chrome Web Store extensions are allowed.
+      bool allowed_extension = extension &&
+          (extension->from_webstore() ||
+          extension->location() == Extension::COMPONENT ||
+          extension->location() == Extension::LOAD);
+
+      // Block any other use of NaCl plugin, unless --enable-nacl is set.
+      if (!allowed_extension && !cmd->HasSwitch(switches::kEnableNaCl)) {
         // TODO(bbudge) Webkit will crash if this is a full-frame plug-in and
         // we return NULL. Prepare a patch to fix that, and return NULL here.
         return CreatePluginPlaceholder(
             render_view, frame, plugin, params, group.get(),
             IDR_BLOCKED_PLUGIN_HTML, IDS_PLUGIN_BLOCKED, false, false);
       }
+
+      // Allow dev interfaces for non-extension apps.
+      bool allow_dev_interfaces = true;
+      if (allowed_extension) {
+        // Allow dev interfaces for component and unpacked extensions.
+        if (extension->location() == Extension::COMPONENT ||
+            extension->location() == Extension::LOAD) {
+          allow_dev_interfaces = true;

[sehr, 2011/10/14 21:38:32]

You initialized allow_dev_interfaces to true, so this is a nop.  You could just
invert the condition and only do the else part.  That would let you merge the
"if (allowed_extension)" part with the inner if also.

[bbudge, 2011/10/14 22:03:27]

Done.

+        } else {
+          // Whitelist extensions from the Chrome Web Store.
+          allow_dev_interfaces =
+              // PDF Viewer plugin
+              (manifest_url.scheme() == "chrome-extension" &&
+              manifest_url.host() == "acadkphlmlegjaadjagenfimbpphcgnh");
+        }
+      }
+      if (allow_dev_interfaces) {
+        std::vector<string16> param_names;
+        std::vector<string16> param_values;
+        param_names.push_back(ASCIIToUTF16("@dev"));
+        param_values.push_back(ASCIIToUTF16(""));
+        AppendParams(
+            param_names,
+            param_values,
+            &params.attributeNames,
+            &params.attributeValues);
+      }
     }
 
     return render_view->CreatePlugin(frame, plugin, params);