Countermeasures for Java/plugin/extension vulnerabilities (disable, warn, offer updates)

RESOLVED FIXED in Firefox 3 alpha8

Status

()

Firefox
Security
P1
critical
RESOLVED FIXED
13 years ago
2 years ago

People

(Reporter: Emmanuel Bourg, Assigned: mwu)

Tracking

Trunk
Firefox 3 alpha8
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.0 -
blocking1.8b5 -
blocking-firefox3 +
blocking1.9a2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:want P1])

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

13 years ago
A Java sandbox vulnerability has been reported against JRE versions prior to
1.4.2_06:

http://secunia.com/advisories/13271/

Even if Mozilla/Firefox is not responsible for this flaw it should adopt a pro
active approach and prevent the browser from running applets with an unsafe
version of the JRE.
(Reporter)

Updated

13 years ago
Flags: blocking-aviary1.0?
Priority: -- → P1
(Reporter)

Updated

13 years ago
Assignee: nobody → security
(Reporter)

Updated

13 years ago
Flags: blocking1.7.5?

Updated

13 years ago
Flags: blocking-aviary1.0? → blocking-aviary1.0-

Updated

13 years ago
Assignee: security → kyle.yuan
Component: Plug-ins → Java: OJI
QA Contact: core.plugins

Comment 1

13 years ago
1.7.5 has shipped. Moving request to 1.7.6.
Flags: blocking1.7.5? → blocking1.7.6?
*** Bug 275429 has been marked as a duplicate of this bug. ***
We can't block arbitrary JRE versions based on our conception of "bad", some
corporate intranet apps require specific internally-approved versions of
software (and correspondingly take on the responsibility for blocking external
rogue content).

This goes beyond Java, any plugin could have known vulnerable versions. Could
this be combined with the plugin finder service somehow? The Update service?
Extensions could have vulnerabilities in them as well.

We should inspect installed components and at the least warn users they have
vulnerable software. Better, we should offer to disable the extension or plugin
(requires at least bug 229196) until the user can upgrade. A link to the
somewhere the user could get more information (vendor's vuln announcement?)
would be nice, as would a link to where users could get an update if one is
available.

And of course it should be possible for corp rollouts to disable the checking or
point it at an internal corporate server.

Not going to get fixed as an OJI bug. Reassigning to Firefox for now, this is
more of a server/UI bug than anything in the "Core" product.
Assignee: kyle.yuan → bugs
Component: Java: OJI → Software Update
Flags: blocking1.7.6? → blocking-aviary1.1?
Product: Core → Firefox
QA Contact: bugs
Summary: Countermeasure for the Java plugin vulnerability → Countermeasures for Java/plugin/extension vulnerabilities (disable, warn)
Whiteboard: [sg:fix]
Flags: blocking-aviary1.1? → blocking-aviary1.1+
*** Bug 288025 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Blocks: 299046
(Reporter)

Updated

12 years ago
No longer blocks: 299046
(Reporter)

Comment 5

12 years ago
*** Bug 299046 has been marked as a duplicate of this bug. ***

Comment 6

12 years ago
This is not a software update bug.  Over to core: security.
Assignee: bugs → dveditz
Component: Software Update → Security
Product: Firefox → Core
QA Contact: bugs → toolkit

Updated

12 years ago
Flags: blocking-aviary1.5+ → blocking1.8b4?

Comment 7

12 years ago
this will have to come in with a patch to 1.5. it requires a policy and work we
don't have yet.
Flags: blocking1.8b4? → blocking1.8b4-

Comment 8

12 years ago
let's go w/ a preference based system, where each plugin dll's filename is a
preference key (use _ to escape . - if a dll has _ in it, deal w/ the conflict,
if someone is annoying enough to ship stupid_plugin.so and stupid.plugin.so,
then only something matching the version we specify will be allowed).

under this system, we can set default preferences specifying the minimum version
number. if a user needs some version that's lower, the user can manually change
the preference to allow the plugin. doing this would be at the user's own risk.

Updated

12 years ago
Flags: blocking-aviary2?

Comment 9

12 years ago
*** Bug 318047 has been marked as a duplicate of this bug. ***
Whiteboard: [sg:fix] → [sg:want P1]

Updated

12 years ago
Flags: blocking-firefox2? → blocking1.8.1?

Comment 10

11 years ago
*** Bug 334790 has been marked as a duplicate of this bug. ***
sample linux java plugin tester is available here - written in a hurry:
http://www.guninski.com/mozbugs/plug-test.html
so i suggest the autoupdater to check for known buggy java/flash via navigator.plugins and if found ask the user if he wants to be sent to secure mozilla.org page for information about update.

an option for turning this off may be nice and probably the luser shouldn't be annoyed more than once a week for the same problem.

Comment 13

11 years ago
Not going to block 1.8.1 for this bug, but we'll happily consider a baked-on-trunk patch.  We should definitely try to do this for A2.
Flags: blocking1.9a2+
Flags: blocking1.8.1?
Flags: blocking1.8.1-
Flags: blocking1.9+

Comment 14

11 years ago
*** Bug 297663 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Summary: Countermeasures for Java/plugin/extension vulnerabilities (disable, warn) → Countermeasures for Java/plugin/extension vulnerabilities (disable, warn, offer updates)
This needs to be fixed at an application level similar to the extension blacklist.
Assignee: dveditz → nobody
Component: Security → Security
Flags: blocking1.9+
Flags: blocking1.8.1-
Product: Core → Firefox
QA Contact: toolkit → firefox
(Assignee)

Updated

10 years ago
Depends on: 330511
(Assignee)

Updated

10 years ago
Assignee: nobody → michael.wu
Target Milestone: --- → Firefox 3 beta1
(Assignee)

Updated

10 years ago
Flags: blocking-firefox3?
(Assignee)

Comment 16

10 years ago
Created attachment 271173 [details] [diff] [review]
Add support for plugins in EM blocklist code, v1

Requires the patch from bug 330511 for the backend support. This hooks into the extensions blocklisting code and allows plugins to be blocklisted based on name, description, or filename, using a regex.
Attachment #271173 - Flags: review?(robert.bugzilla)
Michael, any idea of what it will take to move the plugins / extensions blocklist code out of the EM?
(Assignee)

Comment 18

10 years ago
Created attachment 271766 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support

Would look something like this, and I don't really like it.
Attachment #271766 - Flags: review?(robert.bugzilla)
(Assignee)

Comment 19

10 years ago
Created attachment 272051 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v2

And this one actually works.
Attachment #271766 - Attachment is obsolete: true
Attachment #272051 - Flags: review?(robert.bugzilla)
Attachment #271766 - Flags: review?(robert.bugzilla)
Attachment #271173 - Flags: review?(robert.bugzilla)

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3+

Updated

10 years ago
Whiteboard: [sg:want P1] → [need review rstrong] [sg:want P1]
Missing the freeze, moving out.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Comment on attachment 272051 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v2

>index 0000000..a305055
>--- /dev/null
>+++ b/toolkit/mozapps/extensions/public/nsIBlocklistService.idl
>@@ -0,0 +1,64 @@
>...
>+[scriptable, uuid(0c3fe697-d50d-4f42-b747-0c5855cfc60e)]
>+interface nsIBlocklistService : nsISupports
>+{
>+  /**
>+   * Determine if an item is blocklisted
>+   * @param   id
>+   *          The GUID of the item.
>+   * @param   extVersion
>+   *          The item's version.
How about just version?

>+   * @param   appVersion
>+   *          The version of the application we are checking in the blocklist.
>+   *          If this parameter is undefined, the version of the running
>+   *          application is used.
>+   * @param   toolkitVersion
>+   *          The version of the toolkit we are checking in the blocklist.
>+   *          If this parameter is undefined, the version of the running
>+   *          toolkit is used.
>+   * @returns true if the item is compatible with this version of the
>+   *          application, false, otherwise.
Should be
* @returns true if the item is blocklisted with this version of the
*          application or this version of the toolkit, false, otherwise.

>+   */
>+  boolean isBlocklisted(in AString id, in AString extVersion,
>+                        in AString appVersion, in AString toolkitVersion);
>+};
Since this is only used by the EM managed add-ons how about isAddonBlocklisted?

>diff --git a/toolkit/mozapps/extensions/src/nsBlocklistService.js b/toolkit/mozapps/extensions/src/nsBlocklistService.js
>new file mode 100644
>index 0000000..66d01b2
>--- /dev/null
>+++ b/toolkit/mozapps/extensions/src/nsBlocklistService.js
>@@ -0,0 +1,668 @@
>...
>+ * ***** END LICENSE BLOCK ***** */
#if 0 #endif this comment and preprocess this file to reduce size?
No need to append to the file name .in like was done with the EM.

>+const kELEMENT_NODE                   = Ci.nsIDOMNode.ELEMENT_NODE;
>+const TOOLKIT_ID                      = "toolkit@mozilla.org"
>+const KEY_PROFILEDIR                  = "ProfD";
>+const FILE_BLOCKLIST                  = "blocklist.xml";
>+const PREF_BLOCKLIST_URL              = "extensions.blocklist.url";
Please change this to (apps will need their prefs.js changed)
blocklist.url

>+const PREF_BLOCKLIST_DETAILS_URL      = "extensions.blocklist.detailsURL";
Not needed

>+const PREF_BLOCKLIST_ENABLED          = "extensions.blocklist.enabled";
>+const PREF_BLOCKLIST_INTERVAL         = "extensions.blocklist.interval";
>+const PREF_EM_LOGGING_ENABLED         = "extensions.logging.enabled";
Please change these to (apps will need their prefs.js changed)
blocklist.enabled
blocklist.interval
blocklist.logging.enabled

>+const XMLURI_BLOCKLIST                = "http://www.mozilla.org/2006/addons-blocklist";
Need XMLURI_PARSE_ERROR from the EM as well

>+const MODE_RDONLY   = 0x01;

>+const MODE_WRONLY   = 0x02;
>+const MODE_CREATE   = 0x08;
>+const MODE_APPEND   = 0x10;
>+const MODE_TRUNCATE = 0x20;
>+
>+const PERMS_FILE      = 0644;
>+const PERMS_DIRECTORY = 0755;
>+
>+const CID = Components.ID("{66354bc9-7ed1-4692-ae1d-8da97d6b205e}");
>+const CONTRACT_ID = "@mozilla.org/extensions/blocklist;1"
Should just be
@mozilla.org/blocklist;1

>...
>+/**
>+ * Gets the specified directory at the speciifed hierarchy under a
>+ * Directory Service key.
>+ * @param   key
>+ *          The Directory Service Key to start from
>+ * @param   pathArray
>+ *          An array of path components to locate beneath the directory
>+ *          specified by |key|
>+ * @return  nsIFile object for the location specified. If the directory
>+ *          requested does not exist, it is created, along with any
>+ *          parent directories that need to be created.
>+ */
>+function getDir(key, pathArray) {
>+  return getDirInternal(key, pathArray, true);
>+}
>+
>+/**
>+ * Gets the specified directory at the speciifed hierarchy under a
>+ * Directory Service key.
>+ * @param   key
>+ *          The Directory Service Key to start from
>+ * @param   pathArray
>+ *          An array of path components to locate beneath the directory
>+ *          specified by |key|
>+ * @param   shouldCreate
>+ *          true if the directory hierarchy specified in |pathArray|
>+ *          should be created if it does not exist,
>+ *          false otherwise.
>+ * @return  nsIFile object for the location specified.
>+ */
>+function getDirInternal(key, pathArray, shouldCreate) {
>+  var fileLocator = Cc["@mozilla.org/file/directory_service;1"].
>+                    getService(Ci.nsIProperties);
>+  var dir = fileLocator.get(key, Ci.nsILocalFile);
>+  for (var i = 0; i < pathArray.length; ++i) {
>+    dir.append(pathArray[i]);
>+    if (shouldCreate && !dir.exists())
>+      dir.create(Ci.nsILocalFile.DIRECTORY_TYPE, PERMS_DIRECTORY);
>+  }
>+  dir.followLinks = false;
>+  return dir;
>+}
>+
>+/**
>+ * Gets the file at the specified hierarchy under a Directory Service key.
>+ * @param   key
>+ *          The Directory Service Key to start from
>+ * @param   pathArray
>+ *          An array of path components to locate beneath the directory
>+ *          specified by |key|. The last item in this array must be the
>+ *          leaf name of a file.
>+ * @return  nsIFile object for the file specified. The file is NOT created
>+ *          if it does not exist, however all required directories along
>+ *          the way are.
>+ */
>+function getFile(key, pathArray) {
>+  var file = getDir(key, pathArray.slice(0, -1));
>+  file.append(pathArray[pathArray.length - 1]);
>+  return file;
>+}
Since you there are only calls to getFile how about combining these three into one function?

+/**
+ * Manages the Blocklist. The Blocklist is a representation of the contents of
+ * blocklist.xml and allows us to remotely disable / re-enable blocklisted
+ * items managed by the Extension Manager with an item's appDisabled property.
+ */
+
+function Blocklist() {
+  gApp = Cc["@mozilla.org/xre/app-info;1"].
+         getService(Ci.nsIXULAppInfo);
nit: this looks like it will fit on one line

+  gApp.QueryInterface(Ci.nsIXULRuntime);
+  gPref = Cc["@mozilla.org/preferences-service;1"].
+          getService(Ci.nsIPrefBranch2);
nit: this looks like it will fit on one line

You also need openSafeFileOutputStream... perhaps others?

>...
>+  /**
>+   * The blocklist XML file looks something like this:
>...
>+   * </blocklist> 
>+   */
#if 0 this comment.

>...
>+const gModule = {
>+  registerSelf: function(aCompMgr, aFileSpec, aLocation, aType) {
>+    aCompMgr.QueryInterface(Ci.nsIComponentRegistrar);
>+    aCompMgr.registerFactoryLocation(CID, CLASS_NAME, CONTRACT_ID,
>+                                     aFileSpec, aLocation, aType);
>+
>+    var catMan = Cc["@mozilla.org/categorymanager;1"].
>+                 getService(Ci.nsICategoryManager);
>+    catMan.addCategoryEntry("app-startup", CLASS_NAME, "service," + CONTRACT_ID, true, true);
>+  },
>+
>+  unregisterSelf: function(aCompMgr, aLocation, aType) {
>+    aCompMgr.QueryInterface(Ci.nsIComponentRegistrar);
>+    aCompMgr.unregisterFactoryLocation(CID, aLocation);
>+
>+    var catMan = Cc["@mozilla.org/categorymanager;1"].
>+                 getService(Ci.nsICategoryManager);
>+    catMan.deleteCategoryEntry( "app-startup", "service," + CONTRACT_ID, true);
nit: remove space after (

>diff --git a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>index 7d267e7..f62bf21 100644
>--- a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>+++ b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>...
>@@ -2632,16 +2316,17 @@ function ExtensionManager() {
>     gXPCOMABI = UNKNOWN_XPCOM_ABI;
>   }
>   gPref = Components.classes["@mozilla.org/preferences-service;1"]
>                     .getService(Components.interfaces.nsIPrefBranch2);
> 
>   gOS = Components.classes["@mozilla.org/observer-service;1"]
>                   .getService(Components.interfaces.nsIObserverService);
>   gOS.addObserver(this, "xpcom-shutdown", false);
>+  gOS.addObserver(this, "plugins-list-updated", false);
Not needed.

>@@ -2815,16 +2500,17 @@ ExtensionManager.prototype = {
>     ww.openWindow(null, EMURL, null, EMFEATURES, param);
>   },
> 
>   /**
>    * Clean up on application shutdown to avoid leaks.
>    */
>   _shutdown: function() {
>     gOS.removeObserver(this, "xpcom-shutdown");
>+    gOS.removeObserver(this, "plugins-list-updated");
Not needed

Can you verify the next patch by compiling / running a build with it applied? Please also turn on strict warnings to verify everything it needs is available.
Attachment #272051 - Flags: review?(robert.bugzilla) → review-
On second thought, just leave the pref names and the CONTRACT_ID.
(Assignee)

Comment 23

10 years ago
Created attachment 275847 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v3
Attachment #272051 - Attachment is obsolete: true
Attachment #275847 - Flags: review?(robert.bugzilla)
Comment on attachment 275847 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v3

>diff --git a/toolkit/mozapps/extensions/public/nsIBlocklistService.idl b/toolkit/mozapps/extensions/public/nsIBlocklistService.idl
>new file mode 100644
>index 0000000..17e4f9c
>--- /dev/null
>+++ b/toolkit/mozapps/extensions/public/nsIBlocklistService.idl
>@@ -0,0 +1,64 @@
>...
>+ * The Initial Developer of the Original Code is
>+ * Mozilla Corporation
>+ * Portions created by the Initial Developer are Copyright (C) 2004
s/2004/2007/

>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *  Michael Wu <flamingice@sourmilk.net>  (original author)
nit: additional space before name

>diff --git a/toolkit/mozapps/extensions/src/nsBlocklistService.js b/toolkit/mozapps/extensions/src/nsBlocklistService.js
>new file mode 100644
>index 0000000..bf55ea8
>--- /dev/null
>+++ b/toolkit/mozapps/extensions/src/nsBlocklistService.js
>@@ -0,0 +1,685 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Mozilla Public License Version
>+ * 1.1 (the "License"); you may not use this file except in compliance with
>+ * the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/MPL/
>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is the Blocklist Service.
>+ *
>+ * The Initial Developer of the Original Code is
>+ * Mozilla Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2004, 2007
s/2004, 2007/2007/

>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *  Robert Strong <robert.bugzilla@gmail.com>
>+ *  Michael Wu <flamingice@sourmilk.net>
nit: additional spaces before names

>+ *
>+ * Alternatively, the contents of this file may be used under the terms of
>+ * either the GNU General Public License Version 2 or later (the "GPL"), or
>+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+ * in which case the provisions of the GPL or the LGPL are applicable instead
>+ * of those above. If you wish to allow use of your version of this file only
>+ * under the terms of either the GPL or the LGPL, and not to allow others to
>+ * use your version of this file under the terms of the MPL, indicate your
>+ * decision by deleting the provisions above and replace them with the notice
>+ * and other provisions required by the GPL or the LGPL. If you do not delete
>+ * the provisions above, a recipient may use your version of this file under
>+ * the terms of any one of the MPL, the GPL or the LGPL.
>+ *
>+ * ***** END LICENSE BLOCK ***** */

Per bug 391591... I talked with myk about this and he suggested the following to prevent syntax checking from breaking in komodo.

>+/*
>+ # ***** BEGIN LICENSE BLOCK *****
>+ # ...snip
>+ # **** END LICENSE BLOCK *****
>+ */


>+/**
>+ * Opens a safe file output stream for writing. 
>+ * @param   file
>+ *          The file to write to.
>+ * @param   modeFlags
>+ *          (optional) File open flags. Can be undefined. 
>+ * @returns nsIFileOutputStream to write to.
>+ */
>+function openSafeFileOutputStream(file, modeFlags) {
>+  var fos = Cc["@mozilla.org/network/safe-file-output-stream;1"].
>+            createInstance(Ci.nsIFileOutputStream);
>+  if (modeFlags === undefined)
>+    modeFlags = MODE_WRONLY | MODE_CREATE | MODE_TRUNCATE;
>+  if (!file.exists()) 
>+    file.create(Ci.nsILocalFile.NORMAL_FILE_TYPE, PERMS_FILE);
>+  fos.init(file, modeFlags, PERMS_FILE, 0);
>+  return fos;
>+}
>+
>+/**
>+ * Closes a safe file output stream.
>+ * @param   stream
>+ *          The stream to close.
>+ */
>+function closeSafeFileOutputStream(stream) {
>+  if (stream instanceof Ci.nsISafeOutputStream)
>+    stream.finish();
>+  else
>+    stream.close();
>+}
>+
>+/**
>+ * Constructs a URI to a spec.
>+ * @param   spec
>+ *          The spec to construct a URI to
>+ * @returns The nsIURI constructed.
>+ */
>+function newURI(spec) {
>+  var ioServ = Cc["@mozilla.org/network/io-service;1"].
>+               getService(Ci.nsIIOService);
>+  return ioServ.newURI(spec, null, null);
>+}
>+
>+/**
>+ * Manages the Blocklist. The Blocklist is a representation of the contents of
>+ * blocklist.xml and allows us to remotely disable / re-enable blocklisted
>+ * items managed by the Extension Manager with an item's appDisabled property.
This should be expanded upon to mention that Plugins and how they are disabled.

>+ */
>+
>+function Blocklist() {
>+  gApp = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULAppInfo);
>+  gApp.QueryInterface(Ci.nsIXULRuntime);
>+  gPref = Cc["@mozilla.org/preferences-service;1"].
>+          getService(Ci.nsIPrefBranch2);
>+  gVersionChecker = Cc["@mozilla.org/xpcom/version-comparator;1"].
>+                    getService(Ci.nsIVersionComparator);
>+  gConsole = Cc["@mozilla.org/consoleservice;1"].
>+             getService(Ci.nsIConsoleService);  
>+  
>+}
>+
>+Blocklist.prototype = {
>+  /**
>+   * Extension ID -> array of Version Ranges
>+   * Each value in the version range array is a JS Object that has the
>+   * following properties:
>+   *   "minVersion"  The minimum version in a version range (default = 0)
>+   *   "maxVersion"  The maximum version in a version range (default = *)
>+   *   "targetApps"  Application ID -> array of Version Ranges
>+   *                 (default = current application ID)
>+   *                 Each value in the version range array is a JS Object that
>+   *                 has the following properties:
>+   *                   "minVersion"  The minimum version in a version range
>+   *                                 (default = 0)
>+   *                   "maxVersion"  The maximum version in a version range
>+   *                                 (default = *)
>+   */
This should denote that the above is for _addonEntries and include a section on _pluginEntries

>+  entries: null,
Change to _addonEntries?

>+  _pluginEntries: null,
>+
>+  observe: function (aSubject, aTopic, aData) {
>+    switch (aTopic) {
>+    case "app-startup":
>+      var obsService = Cc["@mozilla.org/observer-service;1"].
>+                       getService(Ci.nsIObserverService);
>+      obsService.addObserver(this, "plugins-list-updated", true);
>+      obsService.addObserver(this, "profile-after-change", true);
Add removeObserver for these

>+
>+  isAddonBlocklisted: function(id, version, appVersion, toolkitVersion) {
>+    if (!this.entries)
>+      this._loadBlocklistFromFile(getFile(KEY_PROFILEDIR, [FILE_BLOCKLIST]));
>+    if (appVersion === undefined)
>+      appVersion = gApp.version;
>+    if (toolkitVersion === undefined)
>+      toolkitVersion = gApp.platformVersion;
>+
>+    var blItem = this.entries[id];
>+    if (!blItem)
>+      return false;
>+
>+    for (var i = 0; i < blItem.length; ++i) {
>+      if (gVersionChecker.compare(version, blItem[i].minVersion) < 0  ||
>+          gVersionChecker.compare(version, blItem[i].maxVersion) > 0)
>+        continue;
>+
>+      var blTargetApp = blItem[i].targetApps[gApp.ID];
>+      if (blTargetApp) {
>+        for (var x = 0; x < blTargetApp.length; ++x) {
>+          if (gVersionChecker.compare(appVersion, blTargetApp[x].minVersion) < 0
>+ ||
Looks like your editor moved some code to the next line

>+              gVersionChecker.compare(appVersion, blTargetApp[x].maxVersion) > 0)
>+            continue;
>+          return true;
>+        }
>+      }
>+
>+      blTargetApp = blItem[i].targetApps[TOOLKIT_ID];
>+      if (!blTargetApp)
>+        return false;
>+      for (x = 0; x < blTargetApp.length; ++x) {
>+        if (gVersionChecker.compare(toolkitVersion, blTargetApp[x].minVersion) <
>+0  ||
same here

>+            gVersionChecker.compare(toolkitVersion, blTargetApp[x].maxVersion) >
>+0)
same here

>...
>+#if 0
>+  /**
>...
Same as comment above

>+   *   <pluginItems>
>+   *     <pluginItem>
>+   *       <match name="name" exp="some plugin"/>
>+   *       <match name="description" exp="1[.]2[.]3"/>
Please provide a few examples. For instance, this infers to me that both name="name" and name="description" are required for a successful match. Also, can this specify that this match is for a specific OS and ABI combination? If not, it probably should be able to. This can be a followup bug if you prefer.

>+   *     </pluginItem>
>+   *   </pluginItems>
>+   * </blocklist> 
>+   */
>+#endif
Same as comment above

r=me with those items fixed and followup bugs filed for the remaining issues you and I have discussed
Attachment #275847 - Flags: review?(robert.bugzilla) → review+
Whiteboard: [need review rstrong] [sg:want P1] → [sg:want P1]
Also, file a bug against AMO to add the capability to blocklist plugins via the blocklist file they serve up
Tests would be handy for this as well. Mossop has done some work for the EM on this in bug 382752
(Assignee)

Updated

10 years ago
Blocks: 391633
(Assignee)

Updated

10 years ago
Blocks: 391714
(Assignee)

Comment 27

10 years ago
Created attachment 276192 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v4

All comments addressed. Will checkin once everything is tested.
Attachment #275847 - Attachment is obsolete: true
Michael, can you resubmit and request another review after addressing the issue for apps that don't use MOZ_PLUGINS? Thanks

Updated

10 years ago
Whiteboard: [sg:want P1] → [sg:want P1][need one more patch]
(Assignee)

Comment 29

10 years ago
Created attachment 276502 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v5

Unbitrotted. No MOZ_PLUGINS ifdefs added since bsmedberg doesn't like it.
Attachment #276192 - Attachment is obsolete: true
Attachment #276502 - Flags: review?(robert.bugzilla)
Comment on attachment 276502 [details] [diff] [review]
Split blocklisting code out, add plugins blocklisting support, v5

You'll need to update packages-static for each app's platform that provides packages-static. Please wait on the landing of the patch in bug 391820.
Attachment #276502 - Flags: review?(robert.bugzilla) → review+
Blocks: 391731
Also, please provide details to morgamic in bug 391633 regarding the format of the blocklist.
No longer blocks: 391731
Blocks: 391731
(Assignee)

Comment 32

10 years ago
Created attachment 276880 [details] [diff] [review]
packages-static changes
(Assignee)

Comment 33

10 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [sg:want P1][need one more patch] → [sg:want P1]
This check in seems to have caused: 
https://bugzilla.mozilla.org/show_bug.cgi?id=392408

Updated

10 years ago
Depends on: 392408

Comment 35

10 years ago
Bug 297663 was duplicated to this one but update function is not available for the plugins, reopening ?

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a8pre) Gecko/2007081605 Minefield/3.0a8pre
(In reply to comment #35)
> Bug 297663 was duplicated to this one but update function is not available for
> the plugins, reopening ?
The original request in Bug 297663 discusses updating for security issues. This bug adds blocklisting for disabling plugins with security issues and there are already other bugs listed in tracking bug 391731 for provide the option to update a plugin with a security problem if the plugin has an update available. So no, do not re-open this bug.

Updated

10 years ago
Blocks: 392385
curses, bitrotted again on bug 299716.
You need to log in before you can comment on or make changes to this bug.