Daniel Veditz [:dveditz] Reporter
	Description • 7 years ago

+++ This bug was initially created as a clone of Bug #1432778 +++

Patches from bug 1432966 plus test breakage fixes landed to fix security bug 1432778 in post-quantum releases. The specific critical escalation path does not exist on the ESR branch but taking the general sanitization fix would protect us against unknown similar issues.

One downside is probable breakage of an unknown number of legacy add-ons, not an issue in the versions that got the original patch.

An alternate, more conservative, approach for ESR might be to finish the audit of that branch (already done? in progress?) and spot-fix any identified bugs. Wouldn't protect against unknown chrome escalation in add-ons, but also wouldn't break a bunch of them. In particular ESR might support a largish number of "Dark Matter" enterprise add-ons in deployments where the "require signing" pref is disabled.

	Frederik Braun [:freddy]
	Comment 1 • 7 years ago

Johann and I audited the ESR branch for potential XSS vulnerabilities, as discussed in the previous meeting.

Yesterday (Monday, Jan 29th) Johann and I went through all occurrences of outerHTML=, innerHTML=, insertAdjacentHTML(), document.write(), document.writeln() and made sure they are listed in the spreadsheet at <https://docs.google.com/spreadsheets/d/1PtTT_9Iz_yHFxN_oRMpkL0ZN1tisdBIn7L_YhtWvbOw/edit#gid=543963657>.
We've omitted uses in test cases and empty string assignments.

All identified source lines are considered secure.

	Mike Conley (:mconley) (:⚙️)
	Comment 2 • 7 years ago

(In reply to Frederik Braun [:freddyb] from comment #1)
> All identified source lines are considered secure.

If that's the case, then it sounds like we've got no spot-fixing to do for now.

It also sounds like what we need to decide here is whether or not landing the patches from bug 1432966 onto ESR is even feasible or desirable given the legacy add-on complication.

	Kris Maglione [:kmag]
	Comment 3 • 7 years ago

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)
> It also sounds like what we need to decide here is whether or not landing
> the patches from bug 1432966 onto ESR is even feasible or desirable given
> the legacy add-on complication.

We can add an exemption to skip sanitization for injections done by extensions without too much trouble. The problem with that, though, is that injections done by add-ons are the most likely ones to be dangerous...

	Mike Conley (:mconley) (:⚙️)
	Comment 4 • 7 years ago

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #3)
> We can add an exemption to skip sanitization for injections done by
> extensions without too much trouble.

I think that's maybe worth exploring, yeah.

> The problem with that, though, is that
> injections done by add-ons are the most likely ones to be dangerous...

True, although I suspect it's strictly better than being vulnerable to something like this via drive-by. Like, the required conditions being that an add-on be installed, enabled, and either be openly hostile or insecurely written, is better than nothing, I suppose.

	Mike Conley (:mconley) (:⚙️)
	Comment 5 • 7 years ago

Hey lizzard, do you or anybody from relman have an opinion on how we proceed here? How much should we really be caring about third-party add-on breakage? And if the answer is "a lot", where does relman stand on the Secure-vs-Working trade-off we seem to be approaching here?

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 7 years ago

The bad addon scenario sounds more moderate-ish in severity? Fixing the critical issue and leaving a moderate one unfixed (especially when it's for compatibility reasons) wouldn't go against our stated policies for security fixes.

	Daniel Veditz [:dveditz] Reporter
	Comment 7 • 7 years ago

Maybe not even "moderate":
 * we've audited our use and not found anything
 * ESR is stable/bugfix-only branch: we don't need to worry about bugs introduced by new features
 * security bugs in add-ons are bugs in add-ons. Nice to protect against if we can, but not a Firefox bug.

Given comment 1 we could think about closing this "worksforme" or "wontfix".

	Johann Hofmann [:johannh]
	Comment 8 • 7 years ago

I'd also consider that we just publicly announced that the threat does not affect ESR (which is true, for all we know). I don't think uplifting this and consequently breaking some add-ons just to be on the safe side is the best way to proceed. ESR 60 is not that far away and we can still move quickly and ignore add-on breakage should we learn about any vulnerabilities we missed.

	Mike Conley (:mconley) (:⚙️)
	Comment 9 • 7 years ago

I concur with johannh. I've been swayed to the WONTFIX camp.

	:Gijs (he/him)
	Comment 10 • 7 years ago

(In reply to Mike Conley (:mconley) (:⚙️) from comment #9)
> I concur with johannh. I've been swayed to the WONTFIX camp.

+1!

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 11 • 7 years ago

My guess is that we'd see a lot of complaint by enterprise-specific add-ons (as Dan mentions in comment 0) so if this is not a critical issue for ESR, I agree we can leave it alone for now and make sure it is completely fixed for 60.

	Mike Conley (:mconley) (:⚙️)
	Comment 12 • 7 years ago

Okay, thanks folks. I hope I'm not being too presumptuous here, but given comment 6 to comment 11, it sounds like this is a WONTFIX, so I'm going to mark it as such. Please re-open if there's other stuff to talk about here.