Closed Bug 662666 Opened 13 years ago Closed 13 years ago

Consider soft-blocking Flash < 10.3.181.22

Categories

(Camino Graveyard :: Plug-ins, defect)

1.9.2 Branch
All
macOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alqahira, Assigned: stuart.morgan+bugzilla)

Details

Attachments

(1 file, 1 obsolete file)

Stuart and I had briefly talked about doing something like this after Flash 10.2 came out and was 10.5+ Intel-only, but at that point the security holes were things like "Flash in an Excel document" that didn't seem particularly web-related nor worth disabling a bunch of the web for ~30% of our users (guesstimate; have to do some data work to get the right numbers).

However, the current security hole clearly affects the web and is being actively exploited to attack at least Gmail[1].  Reading the Adobe bulletin[2] certainly makes it sound like all versions of Flash are vulnerable, and of course no updated Flash is forthcoming for 10.5 PPC or 10.4 users.

Given that, I think we should consider soft-blocking all versions of Flash older than the current 10.3.181.22 in Camino 2.1; anyone on 10.5 PPC or 10.4 should be able to re-enable Flash (how?) if they feel using it to be able to see YouTube or site X merits the risk.

I'm also tempted to ask how hard it would be to backport the blocklist part of the patch to 2.0.x, since that's where most of our users are right now :-(

[1] http://www.macworld.com/article/160330/2011/06/flash_gmail_bug.html
[2] http://www.adobe.com/support/security/bulletins/apsb11-13.html
Flags: camino2.1b1?
We could block on Intel, and just flip the pref on on PPC.

I don't remember how complex the code was or what it relied on. Can we ship 2.1 instead? ;)
(In reply to comment #1)
> We could block on Intel, and just flip the pref on on PPC.

Which pref?  The general "disable arbitrary things" one?  That gets us a "missing" UI (in the best case[1], and no UI at all in 90% of the cases--just whatever the site chooses to do in its detection routines) rather than a "blocked" one, and we really want to show 10.5 PPC and all 10.4 users a scary warning message of some sort (via the link to cbo).

Flipping the pref would allow easy re-enabling for users on those versions willing to take the risk, certainly.

And what about on 10.4 Intel, which also has no safe version?

I'm fine with (hard-)blocking on 10.5 Intel and 10.6, though.

[1] http://www.adobe.com/software/flash/about/
[2] http://www.youtube.com/ http://browserspy.dk/flash.php
Sorry, I was thinking of the Flashblock pref. It's not exactly an ideal security approach, but it has the advantage that it would be less internet-breaking for people, so they might be more inclined to leave it set and just click through on specific sites, rather than going back to being wildly unsafe. I'd be fine doing the same on 10.4 (I forgot that's now doomed as well).

Agreed on hard-blocking on newer OSes.
So, apparently I became confused, because http://www.adobe.com/products/flashplayer/systemreqs/ says 10.4 Intel is supported (love how the table says "10.5" and "10.4 (Intel)", but then has a note "PowerPC® based computers are not supported with Flash Player 10.3." after the footnotes :P ).

So, let's just do what you proposed.  I'd really like to show some sort of warning to the PPC users on upgrade to explain what we've done and specifically why, but I'd like to get the blocking/protection working more than that ;)
Assignee: nobody → stuart.morgan+bugzilla
Flags: camino2.1b1? → camino2.1b1+
Attached patch fix (obsolete) — Splinter Review
This does a few things:
- Hard-blocks everything but the current latest Flash for both Intel and PPC (the respective latest versions)
- Adds a hidden pref to turn that off, since making it impossible (or even just really hard, like hand-editing pluginreg.dat) to load all those older versions makes me nervous (e.g., what about regression testing?). We'll have to document that changing the pref will require also quitting Camino and nuking pluginreg.dat.
- Turns on Flashblock on PPC
- Changes the language for the blocked plugin text, so it mentions security. It's longer, but I don't see any way around that.

Explanatory UI for PPC would be nice, but a modal dialog at startup seems really harsh, and I don't really want to fiddle with adding anything more complicated. We'll relnote it, and see how it goes in beta :)

Caveat: since the blocking is based on the blocklist service, it relies on pluginreg.dat being rebuilt to kick in. Luckily we already have to the code to force that on autoupdate, so we'll get most people.
Attachment #542015 - Flags: feedback?(alqahira)
Thanks for picking this up, Stuart (I had started to look at it last weekend, but got hung up on the PPC-specific stuff and having to rework all the version-related code to handle the 4th value).

A couple of quick comments based on the info provided and a quick skim of the patch:

(In reply to comment #5)
> - Adds a hidden pref to turn that off, since making it impossible (or even
> just really hard, like hand-editing pluginreg.dat) to load all those older
> versions makes me nervous (e.g., what about regression testing?). We'll have
> to document that changing the pref will require also quitting Camino and
> nuking pluginreg.dat.

Good catch.  Since the intention here, it seems, is for developer/debugging-type activities, can we bikeshed the name to "camino.debug_allow_dangerous_plugins" to further discourage any random flipping of said pref?

> - Turns on Flashblock on PPC

>+  if (mLastRunPrefsVersion && mLastRunPrefsVersion < 5) {
>+#if defined(__ppc__)
>+    // PPC users don't have access to a version of Flash without serious known
>+    // vulnerabilities, so enable Flash blocking to help increase security.
>+    [self setPref:kGeckoPrefBlockFlash toBoolean:YES];
>+#endif
>+  }

Is this going to do the right thing on new profiles on PPC?  I don't think so, based on your bug 632278 comment 5 review comment to me.  Since there's nowhere else that we're setting the Flashblock pref for PPC (e.g., we'd normally set the new default in all-camino.js, but that file can't differ per-arch or handle arch-specific differences), there's no other default for new PPC profiles to inherit, leaving them without this mitigation.  Can we drop "mLastRunPrefsVersion &&" to make it run unconditionally on PPCs, or do we need some other construct?

It still won't do the right thing for people who might shuffle profiles back and forth between PPC and Intel and upgrade to v5 on Intel, but I can't imagine that there are many of these, if any, whereas creating a new profile is still fairly common (and we, astoundingly, continue to get new users).

> - Changes the language for the blocked plugin text, so it mentions security.
> It's longer, but I don't see any way around that.

Seems OK.

> Caveat: since the blocking is based on the blocklist service, it relies on
> pluginreg.dat being rebuilt to kick in. Luckily we already have to the code
> to force that on autoupdate, so we'll get most people.

This really bothers me now that we're actually using the blocklist for security.  Can we make PreferenceManager consult the blocklist on the first run of every build (similar to how we prelaunch feedhandlers in new builds), so that we do get everybody, whether they're auto-updating or not?

I'll try to take the patch for a spin this evening.
(In reply to comment #6)
> Good catch.  Since the intention here, it seems, is for
> developer/debugging-type activities, can we bikeshed the name to
> "camino.debug_allow_dangerous_plugins" to further discourage any random
> flipping of said pref?

Well, there are other plausible cases; e.g., some Flash security update also breaks some bizarre edge-case configuration, so some people have to downgrade temporarily. I've actually seen a couple bugs like this in the Chromium bug tracker (granted they were Linux IIRC, but still).

I think having "dangerous" in the name is about as discouraging as we can get anyway; if that doesn't stop them, "debug" won't either. And we'll put bold red blink text on the hidden prefs page ;)

> Is this going to do the right thing on new profiles on PPC?

Nope, copy-paste for the fail. Thanks.

> This really bothers me now that we're actually using the blocklist for
> security.  Can we make PreferenceManager consult the blocklist on the first
> run of every build (similar to how we prelaunch feedhandlers in new builds),
> so that we do get everybody, whether they're auto-updating or not?

It's Core consulting the blocklist that causes the plugin to be blocked. We couldn't find a way to trigger that from the outside, which is why we do the manual file deleting.

We might be able to rig something up to where we could detect this case and then set a flag to have us nuke the pluginreg.dat file on the next quit, so it would get fixed on the second launch. Let's file a new bug for that though; I'd have to think through how to be sure we wouldn't do it too much, and I don't think it's a must-have for b1. I'd like to land this and then get b1 out the door.
Attached patch v2Splinter Review
Fixes the enable-Flash-blocking check.
Attachment #542015 - Attachment is obsolete: true
Attachment #542015 - Flags: feedback?(alqahira)
Attachment #542040 - Flags: feedback?
Attachment #542040 - Flags: feedback? → feedback?(alqahira)
Comment on attachment 542040 [details] [diff] [review]
v2

On the plus side, with the new string "Learn" just barely fits inside the Adobe "What's my Flash version" object's space when Flash is blocklisted, so users will be able to click on the link for our "more info" in something that small.

On the negative side, everyone that does Flash detection either before (Adobe elsewhere) or after (Youtube) the fact prevents users from seeing our UI, which is unfortunate.

(In reply to comment #7)
> It's Core consulting the blocklist that causes the plugin to be blocked. We
> couldn't find a way to trigger that from the outside, which is why we do the
> manual file deleting.

Ugh.  Firefox apparently has some way to trigger these checks at startup (via the Extensions Manager and the EM restart), but I wasn't able to track it down in the amount of time it seemed worthwhile to spend on trying to do so.

Can we not have PreferenceManager call |GetPluginBlocklistState| on Flash/other plug-ins to "force" an update?  (The fact the plug-ins code only calls the blocklist service/|GetPluginBlocklistState| if the plug-in info is not already cached is annoying.)

nsBlocklistService.js looks like it has a private copy of |GetPluginBlocklistState| that it apparently uses to communicate blocklist.xml blocklist entries to the service: http://bonsai-hg.konigsberg.mozilla.org/bonsai/cvsblame.cgi?file=/toolkit/mozapps/extensions/src/nsBlocklistService.js&rev=c846b94321ee&tree=mozilla1.9.2&mark=715-768#707

Related: the "camino.allow_dangerous_plugins" pref doesn't work for similar reasons, so the workflow would have to be something like this to do any good:
1) Flip the pref while Camino is running with a secure Flash installed
2) Quit Camino
3) Downgrade Flash
4) Restart Camino

If you've already downgraded Flash before you flip the pref in Camino, you're stuck in Gecko plug-in caching hell. :-(

So we really need to figure *something* out; if not here, then in a new bug.

>@@ -116,6 +130,14 @@ NS_IMETHODIMP PluginBlocklistService::GetPluginBlocklistState(nsIPluginTag *plug
> 
>   BOOL blocked = NO;
>   if ([name hasPrefix:kPluginNameFlash]) {
>+    // Unless the user allows dangerous plugins, disable versions of Flash with
>+    // known security vulnerabilities.
>+    BOOL prefLoaded = NO;
>+    BOOL allowDangerousPlugins = [[PreferenceManager sharedInstanceDontCreate]
>+        getBooleanPref:kGeckoPrefAllowDangerousPlugins withSuccess:&prefLoaded];
>+    if (prefLoaded && !allowDangerousPlugins)
>+      blocked = IsOlder(version, minFlashVersion);
>+
>     // Flash 9 leaks file handles on every instantiation.
>     if (version.major == 9)
>       blocked = YES;

Is the Flash 9 code still needed now that we're blocking < 10.1.x/10.3.x? Or do we still want to prevent users from using Flash 9 even if they choose to allow dangerous plug-ins?

Other than the caching issue with the pref, and with the other big caveat that I haven't tested on PPC, this seems OK.

> I think having "dangerous" in the name is about as discouraging as we can
> get anyway; if that doesn't stop them, "debug" won't either. And we'll put
> bold red blink text on the hidden prefs page ;)

Given that this is very specialized, I'd rather have it only on the wiki.

f=ardissone with responses/new bugs as appropriate.
Attachment #542040 - Flags: feedback?(alqahira) → feedback+
(In reply to comment #9)
> Can we not have PreferenceManager call |GetPluginBlocklistState| on
> Flash/other plug-ins to "force" an update?

Nope; that would let PreferenceManager know what we think should happen, but it wouldn't actually update anything. The thing that does the updating of the real plugin state is Core code that calls GPBS.

> Related: the "camino.allow_dangerous_plugins" pref doesn't work for similar
> reasons, so the workflow would have to be something like this to do any good:
> 1) Flip the pref while Camino is running with a secure Flash installed
> 2) Quit Camino
> 3) Downgrade Flash
> 4) Restart Camino

No, like I said earlier they just need to nuke pluginreg.dat at step 3. I think for a pref we don't want to be used except in extreme cases that's fine.

> Is the Flash 9 code still needed now that we're blocking < 10.1.x/10.3.x? Or
> do we still want to prevent users from using Flash 9 even if they choose to
> allow dangerous plug-ins?

That was my thought, yes. For the really bad stability cases I don't think we want the pref to apply.

File bug 667441 for better state refreshing.
(In reply to comment #10)
> (In reply to comment #9)
> > Can we not have PreferenceManager call |GetPluginBlocklistState| on
> > Flash/other plug-ins to "force" an update?
> 
> Nope; that would let PreferenceManager know what we think should happen, but
> it wouldn't actually update anything. The thing that does the updating of
> the real plugin state is Core code that calls GPBS.

…but only calls GPBS if the plug-in has changed :P  Yay.  OK.

> No, like I said earlier they just need to nuke pluginreg.dat at step 3. I
> think for a pref we don't want to be used except in extreme cases that's
> fine.

Yeah, that's fine; I seem to have forgotten last night that deleting pluginreg.dat gets you out of plug-in caching hell.

> > Is the Flash 9 code still needed now that we're blocking < 10.1.x/10.3.x? Or
> > do we still want to prevent users from using Flash 9 even if they choose to
> > allow dangerous plug-ins?
> 
> That was my thought, yes. For the really bad stability cases I don't think
> we want the pref to apply.

OK, sounds good.
Landed as http://hg.mozilla.org/camino/rev/ffb9c8f56829 (with the Flash version rev'd since of course there's been another critical security release since I wrote the patch).
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
(In reply to comment #12)
> Landed as http://hg.mozilla.org/camino/rev/ffb9c8f56829 (with the Flash
> version rev'd since of course there's been another critical security release
> since I wrote the patch).

Was the last one really a critical security fix?  I never saw a bulletin for it, so I never knew what was up with it.
Whoops, it wasn't; I misremembered. Fixed with http://hg.mozilla.org/camino/rev/bd9315459ab3
Is our plan here to rev the blocked version every time there's a Flash update with a critical security fix (which seems to be almost every one)?

The latest version isn't fixing 0-days this time (as the updates that triggered filing this bug were), but the details ("DETAILS" section) in the advisory don't indicate much, other than presumably it's not just a "Flash-in-Excel"-type vuln this time: http://www.adobe.com/support/security/bulletins/apsb11-21.html
Yeah, I've been wondering the same thing. I think I'd be okay with that, but it means more annoying bookkeeping, and it could get to be annoying for users to have Flash stop working whenever they update Camino. We might be better off only blocking for the really egregious cases, like 0-days.
Things like this:
http://www.reddit.com/r/starcraft/comments/je734/new_adobe_flash_plugin_breaks_justin_tv/
may be an argument for the latter.
Heh.

It would be nice, I think, at some point to periodically move the bar up to "known good" critical update versions that have been available a bit (even more record-keeping! :( ), but I guess for now the safest thing is to just rev the block for 0-days and other similarly-egregious issues.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: