1353204 - ContentPrincipal::GenerateOriginNoSuffixFromURI should not return the hostport in the ORIGIN_IS_FULL_SPEC case

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Bobby Holley (:bholley)]

bz pointed out that this code has some weird behavior that doesn't match my recollection.

What I _thought_ we did was to only return a string from GetOrigin if either:
(a) the URI scheme is whitelisted, or
(b) the URI QIs to nsStandardURL.

But we also appear to return something if GetAsciiHostPort doesn't fail. This is demonstrably wrong in the ORIGIN_IS_FULL_SPEC case, but in general is a lot more loose than I'd like.

Boris, do you think we can get away with just requiring nsIStandardURL in this case? I think we can, since the only in-tree implementation of GetAsciiHostPort that doesn't fail and isn't nsIStandardURL is moz-icon [1]. 

This is potentially a more serious issue now that we're relying on GetOrigin more in bug 1347817, though in practice I think it only affects addons.

[1] http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/image/decoders/icon/nsIconURI.cpp#515

Comment 1

[Nika Layzell [:nika] (ni? for response)]

I also remember us requiring nsStandardUri... I'll have to look at the code again tomorrow.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> since the only in-tree implementation of GetAsciiHostPort that doesn't fail and isn't nsIStandardURL is moz-icon

This is demonstrably false unless you count Thunderbird as "not in tree".  Which you should not: a bunch of this security infra was _specifically_ designed with Thunderbird's needs in mind.

Plus of course extensions.  Both Firefox and Thunderbird ones....

I would like to see a specific proposal for what the behavior should be here.  Obviously just requiring nsIStandardURL without making any other changes to either this code or Thunderbird is a non-starter.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #2)
> > since the only in-tree implementation of GetAsciiHostPort that doesn't fail and isn't nsIStandardURL is moz-icon
> 
> This is demonstrably false unless you count Thunderbird as "not in tree". 
> Which you should not: a bunch of this security infra was _specifically_
> designed with Thunderbird's needs in mind.

It's sure hard to discover that with comm-central being in a separate repo, alongside the fact that we're not supposed to spend time worrying about Thunderbird.

> Plus of course extensions.  Both Firefox and Thunderbird ones....

I don't see any Firefox extensions that define asciiHostPort. Given that ORIGIN_IS_FULL_SPEC is only defined for Thunderbird, it seems like this wouldn't affect Firefox extensions.

> I would like to see a specific proposal for what the behavior should be
> here.  Obviously just requiring nsIStandardURL without making any other
> changes to either this code or Thunderbird is a non-starter.

My proposal would be:
(1) For Thunderbird, whitelist any URI that has ORIGIN_IS_FULL_SPEC.
(2) Get rid of the hostport check, and require nsIStandardURL modulo the whitelisted cases.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> It's sure hard to discover that with comm-central being in a separate repo

True, but we do have code search for it...

> alongside the fact that we're not supposed to spend time worrying about Thunderbird.

I don't think that's reasonable as stated.  We're not supposed to spend time on fixing their tree, certainly, but I do think we should give them a heads-up on fundamental changes, and we are probably NOT supposed to introduce Thunderbird security bugs by breaking core code invariants they depend on without telling them something about it.

> Given that ORIGIN_IS_FULL_SPEC is only defined for Thunderbird, it seems like this wouldn't affect Firefox
> extensions.

The question was about things that have a GetAsciiHostPort that returns something but that aren't nsIStandardURL.  That's orthogonal to the ORIGIN_IS_FULL_SPEC thing.

Now we could say we don't care about the non-core-Thunderbird instances which have such a GetAsciiHostPort.  I'm just saying that I don't know whether we do or not, and we should at least check with someone who might know something about the Thunderbird URI setup to see whether we do.

> My proposal would be:

OK, so we require nsIStandardURL and then what?  Presumably still try the hostport (but only if implementing nsIStandardURL) and fall back to full spec if that fails, right?

My question is what this buys us over the current setup where we try hostport if it's supported at all (except we shouldn't do that in the FULL_SPEC case).  I'd like to understand what the benefit we're trying to gain by excluding some unknown set of current URI implementations is.

Note, by the way, that afaict moz-icon URIs in fact fail out on GetAsciiHostPort.  So for in-Firefox-tree purposes, the assumption that GetAsciiHostPort doesn't fail if and only if nsIStandardURL was not an unreasonable one.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #4)
> The question was about things that have a GetAsciiHostPort that returns
> something but that aren't nsIStandardURL.  That's orthogonal to the
> ORIGIN_IS_FULL_SPEC thing.
> 
> Now we could say we don't care about the non-core-Thunderbird instances
> which have such a GetAsciiHostPort.  I'm just saying that I don't know
> whether we do or not, and we should at least check with someone who might
> know something about the Thunderbird URI setup to see whether we do.

I really don't know anything about the Thunderbird security model or its addon ecosystem. I'm happy to take suggestions from somebody who does. Who is that person?

If the answer is "bz and only bz", well, that sucks and I'm sorry - that's a pretty annoying albatross to be stuck with. But please understand that I don't know anything about Thunderbird, and it's not really practical for me to change that in the foreseeable future. So unless you want me to get your review on any significant changes that come through any of my possibly-thunderbird-affecting-modules, we're going to be stuck with the current reactive workflow. 

> 
> > My proposal would be:
> 
> OK, so we require nsIStandardURL and then what?  Presumably still try the
> hostport (but only if implementing nsIStandardURL) and fall back to full
> spec if that fails, right?

If we require nsIStandardURL, GetAsciiHostPort should never fail AFAICT.

> My question is what this buys us over the current setup where we try
> hostport if it's supported at all (except we shouldn't do that in the
> FULL_SPEC case).  I'd like to understand what the benefit we're trying to
> gain by excluding some unknown set of current URI implementations is.

The benefit would be that we have a stronger guarantee that the serializations do not contain "^", which would violate our assumptions of the origin stringification. This is important because we're now using that stringification for all security checks, not just some of them.

Again, totally happy to have whatever thunderbird-specific code is needed for their stuff to work, as long as it's #ifdef-ed out of Firefox builds.

> Note, by the way, that afaict moz-icon URIs in fact fail out on
> GetAsciiHostPort.  So for in-Firefox-tree purposes, the assumption that
> GetAsciiHostPort doesn't fail if and only if nsIStandardURL was not an
> unreasonable one.

Oh, I didn't see that. Combined with my check for Firefox addons above, I think this isn't an active security issue in Firefox. Who's radar should this be on? Jorge's?

Comment 6

[Boris Zbarsky [:bzbarsky]]

I think the reactive workflow is the best we can do here.  I have no idea who's even actively involved in Thunderbird, much less who knows stuff about the security model.  Maybe Jorg knows who would be the right contacts?  Jorg, please cc the people who are responsible for Thunderbird security stuff, if you know who they are.

To be clear, I know a little about the security model and nothing about the addon ecosystem for Thunderbird.  I agree it would be best if someone who does know something about it were on hand to respond to questions along the lines of "will this break you?".  If we can't find someone to do something this basic, we should immediately announce Thunderbird end of life, because shipping an insecure mail client is much worse than shipping no mail client at all.  :(

> If we require nsIStandardURL, GetAsciiHostPort should never fail AFAICT.

"fails" in this code includes "returns an empty string".  It's totally possible to have an nsIStandardURL with empty hostport.  file:// does this all the time, for example.

> The benefit would be that we have a stronger guarantee that the serializations do not contain "^"

I don't think there's any such guarantee (though I guess it is _stronger_ in the sense of the cases in which it's missing being smaller).  Implementing nsIStandardURL doesn't guarantee that, obviously; what guarantees that is the actual implementation.

What I think we might have is that the in-Firefox-tree implementations of nsIStandardURL happen to provide such a guarantee.  But do they?  '^' is not obviously supposed to be %-encoded per the URL spec as far as I can tell.  In servo, this testcase:

  <a href="http://example.com/foo^bar">Test</a>
  <script>
    alert(document.querySelector("a").href);
  </script>

shows "http://example.com/foo^bar".  That's also what Safari shows (though Chrome %-encodes like Firefox does right now).  I haven't tried the rust-url Gecko thing; I don't know what it actually does.  Does it behave more like Servo's URL impl, or more like Gecko's nsStandardURL and why?  Per my reading of the URL spec right now, the '^' should be left as-is in this case, by the way.

> Who's radar should this be on?

In what sense?  Obviously the Thunderbird team's, right?  Apart from that, ORIGIN_IS_FULL_SPEC support is not compiled into Firefox at all, so only Thunderbird addons might be affected, if any addons are affected.

Comment 7

[Jorg K (CEST = GMT+2)]

So the question is here, who's on the TB security team? Well, you can find the core developers here:
https://wiki.mozilla.org/Thunderbird/Core_Team
Sadly, our Mailnews cracks Joshua Cranmer and Neil Rashbrook have left the building, so it's me Magnus and Kent looking into security issues.

Comment 8

[Bobby Holley (:bholley)]

In our call, bz and I settled on the following order of operations for this function:

(1) Check ORIGIN_IS_FULL_SPEC
(2) Check whitelist
(3) Check withprincipal case
(4) Reject non-nsIStandardURL
(5) Return hostport if nonempty
(6) Return full spec


I'll write a patch.

Comment 9

[Bobby Holley (:bholley)]

Attachment: Part 1 - Factor out the full-spec-to-origin machinery and use it if the Thunderbird-only flag is set. v1

MozReview-Commit-ID: 8acGT0n6O3X

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 2 - Move the hostPort case after the nsIStandardURL check. v1

MozReview-Commit-ID: HIqxhEE41G0

Comment 11

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea0a4cbb16f946984444bebc786827f5b08476ae

Comment 12

[Bobby Holley (:bholley)]

These patches should also fix bug 1352701. NI jorg to confirm.

Comment 13

[Jorg K (CEST = GMT+2)]

Thank you very much, this works fine for me.

Sorry about changing the summary of bug 1352701. Since the action is here now, we're using the other bug for writing a test.

Comment 14

[Bobby Holley (:bholley)]

Ok great. Try push is green, so we should be good to go modulo any issues raised in review.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8854588 [details] [diff] [review]
Part 1 - Factor out the full-spec-to-origin machinery and use it if the Thunderbird-only flag is set. v1

r=me

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8854589 [details] [diff] [review]
Part 2 - Move the hostPort case after the nsIStandardURL check. v1

s/See we/see whether we/

>+  // XXXbholley: Should chrome:// URLs just return an empty string for hostPort.

s/hostPort./hostPort,/

I think that would break code that currently uses host stuff on chrome:// to get the chrome package...

r=me

Comment 17

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/autoland/rev/b75160f48d6023cb8f3eccaeb987f051f778668c
remote:   https://hg.mozilla.org/integration/autoland/rev/cfcd60938aa0916ee887a30cf89f88c2d46495db

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b75160f48d60
https://hg.mozilla.org/mozilla-central/rev/cfcd60938aa0

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Does this need backport? If so, how far? Note that we're building 53b10 today and the RC on Monday.

Comment 20

[Jorg K (CEST = GMT+2)]

The fix here was made necessary by bug 1347817 which landed on mozilla55, so as far as I can see, no uplift required at all.

Comment 21

[Boris Zbarsky [:bzbarsky]]

> The fix here was made necessary by bug 1347817 which landed on mozilla55

Not quite.  Bug 1347817 is what made this problem blindingly obvious.  The problem was preexisting, to the extent that anything relied on origin strings.

Ryan, this security bug is effectively Thunderbird-specific, albeit introduced by core changes.  So it doesn't affect any Firefox version at all.  I'm not sure whether we should move it to Thunderbird, given that it _did_ involve a core code change, or how best to track this...

Anyway, in terms of backports, it should probably be backported to whatever our supported Thunderbird branches are, at a guess.

Comment 22

[Jorg K (CEST = GMT+2)]

Boris, which bugs would require backporting and to where? It can't be bug 1347817 which really only landed on mozilla55.

If required, I can uplift anything to our THUNDERBIRD_52_VERBRANCH. That's a branch on mozille-esr52. For beta versions, we can also create a release branch.

Comment 23

[Boris Zbarsky [:bzbarsky]]

> Boris, which bugs would require backporting and to where?

The fix from this bug (part 1 only), and to whatever Thunderbird branches exist that we plan to ship a release off to users, imo.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

I could see maybe uplifting to Aurora still. Maybe ESR52 if we think it'll reduce the likelihood of later backport conflicts given how new of a branch it is. I'd say we're better off avoiding the churn on Beta at this point given that we're building the RC early next week.

Comment 25

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #23)
> > Boris, which bugs would require backporting and to where?
> 
> The fix from this bug (part 1 only), and to whatever Thunderbird branches
> exist that we plan to ship a release off to users, imo.

Only those releases with bug 1347817, right? This patch wouldn't apply otherwise.

Comment 26

[Bobby Holley (:bholley)]

Though I guess we could patch the equivalent code on previous branches that computes the origin string. The question is whether any Thunderbird security invariants happen to depend on any code that uses the stringification for comparison (which was possible but less common before bug 1347817).

Comment 27

[Jorg K (CEST = GMT+2)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #25)
> Only those releases with bug 1347817, right? This patch wouldn't apply
> otherwise.
Since bug 1347817 landed on mozilla55 without uplifts, there are no "releases" that contain it. What am I missing?
So there are problems even before bug 1347817? I think that's what Boris is telling us, but since there were no *visible* problems, no one noticed.

Comment 28

[Boris Zbarsky [:bzbarsky]]

> Only those releases with bug 1347817, right?

No.  For releases before bug 1347817 the patch needs actual backporting, of course, not just application...

> The question is whether any Thunderbird security invariants happen to depend on any
> code that uses the stringification for comparison 

Right.  I suspect doing that audit is more work than backporting the changes to GetOrigin.

Comment 29

[Bobby Holley (:bholley)]

I don't have time to manage the backport. Jorg, can you uplift part 1 to whatever branches you need and flag me for review?

Comment 30

[Jorg K (CEST = GMT+2)]

Attachment: aurora.patch: Part 1 for M-A and M-B

Comment 31

[Jorg K (CEST = GMT+2)]

Comment on attachment 8855975 [details] [diff] [review]
aurora.patch: Part 1 for M-A and M-B

Same refactoring you had. Should be pretty risk-free to uplift to M-A since it's merely refactoring and adding code not compiled for FF.

ERS52 version to follow.

Comment 32

[Jorg K (CEST = GMT+2)]

Looking at the code a little closer while making it apply to M-A and M-B I'm wondering about one thing:

The function is called AssignFullSpecToOriginNoSuffix() but you're not only stripping the ref part (#) but also the query part (?):
https://hg.mozilla.org/mozilla-central/rev/b75160f48d60#l1.21

So we're not getting the full spec after all, right?

The point was that some message URLs only differ in the query part, for example:
mailbox:///path/to/file/folder?number=1 and
mailbox:///path/to/file/folder?number=2.

So messages in the same folder get the same origin, right? It's certainly better then all messages getting the same origin, but I'm not sure that what Boris intended, although he reviewed it. Or am I missing something?

Comment 33

[Jorg K (CEST = GMT+2)]

Oh, I forgot to say thank you, so: Thank you.

Comment 34

[Bobby Holley (:bholley)]

(In reply to Jorg K (GMT+2) from comment #32)
> So messages in the same folder get the same origin, right? It's certainly
> better then all messages getting the same origin, but I'm not sure that what
> Boris intended, although he reviewed it. Or am I missing something?

Good point - wdyt boris?

(In reply to Jorg K (GMT+2) from comment #33)
> Oh, I forgot to say thank you, so: Thank you.

You're welcome!

Comment 35

[Boris Zbarsky [:bzbarsky]]

Ugh.  You're not missing anything.

We need to have the same semantics here as NS_SecurityCompareURIs.  So yes, we want to include the query in the string we produce, though it's probably OK to strip out the ref if we really want to...

I'm going to reopen this, because that will simplify backports as opposed to creating a new bug to fix this.

God, I wish Thunderbird actually had tests for its security stuff.  :(

Comment 36

[Jorg K (CEST = GMT+2)]

Attachment: 1353204-Part1-follow-up.patch

Wouldn't it just be a matter of reversing Part 1 apart from leaving the hunk for TB? What am I missing?

Comment 37

[Jorg K (CEST = GMT+2)]

Attachment: 1353204-Part1-follow-up.patch

Sorry about the noise, fixed the commit message to make it decent and avoid r- ;-)

Comment 38

[Magnus Melin [:mkmelin]]

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #35)
> God, I wish Thunderbird actually had tests for its security stuff.  :(

We do.e.g. https://dxr.mozilla.org/comm-central/source/mail/test/mozmill/content-policy, but maybe not as extensive as we should.

Comment 39

[Jorg K (CEST = GMT+2)]

Thanks for the f+, but someone has to convert the f+ to an r+ so this can land ;-) - Wait for Boris?

Comment 40

[Bobby Holley (:bholley)]

Yeah I'd like bz to sign off, since I don't know the TB model.

Comment 41

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8856090 [details] [diff] [review]
1353204-Part1-follow-up.patch

r=me

Comment 42

[Jorg K (CEST = GMT+2)]

Hi Sheriff, this bug has become a little confusing. What needs landing here is 1353204-Part1-follow-up.patch.

The other two patches landed in comment #18:
https://hg.mozilla.org/mozilla-central/rev/b75160f48d60
https://hg.mozilla.org/mozilla-central/rev/cfcd60938aa0

Thanks as always for your help.

Comment 43

[Jorg K (CEST = GMT+2)]

Attachment: aurora.patch: Part 1 for M-A and M-B and M-esr52

Here is the updated Aurora/Beta/ESR patch.

Comment 44

[Jorg K (CEST = GMT+2)]

Just out of interest, which spec does the system use without this hunk in Aurora/Beta/ESR?

Comment 45

[Carsten Book [:Tomcat]]

(In reply to Jorg K (GMT+2) from comment #42)
> Hi Sheriff, this bug has become a little confusing. What needs landing here
> is 1353204-Part1-follow-up.patch.
> 
> The other two patches landed in comment #18:
> https://hg.mozilla.org/mozilla-central/rev/b75160f48d60
> https://hg.mozilla.org/mozilla-central/rev/cfcd60938aa0
> 
> Thanks as always for your help.

np, done in https://hg.mozilla.org/integration/mozilla-inbound/rev/5a07a004ca309f405dd7b20ba57ab224aae494b5

Comment 46

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5a07a004ca30

Comment 47

[Jorg K (CEST = GMT+2)]

Comment on attachment 8856364 [details] [diff] [review]
aurora.patch: Part 1 for M-A and M-B and M-esr52

Approval Request Comment
[Feature/Bug causing the regression]: I don't know, see below.
[User impact if declined]: Security problem in TB, see below.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes, TB Daily.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not for Firefox.
[Why is the change risky/not risky?]: Not risky since the code is only compiled for Thunderbird.
[String changes made/needed]: None.

Sorry, I can't name the regressing bug and what the impact on TB is. I'm asking again:
Without this patch for TB, which spec would be used? - Comment #44.
If not the full spec, when did that change, so what is the regressing bug?
I also need to know in order to uplift this accordingly. Well, looking at
https://hg.mozilla.org/releases/mozilla-esr52/file/tip/caps/nsPrincipal.cpp
it appears that it fix is needed there, too.

With 55 fixed, uplift for 54 requested, TB 53 beta done and won't be repeated, another uplift option is TB 52 ESR where the patch also applies. But it would really be nice to understand which reshuffling caused the full spec not to be used in the first place.

Particularly comment #21 (quote):
  The problem was pre-existing, to the extent that anything relied on origin strings.

Boris, I tried to did through some history here, maybe it came from here:
https://hg.mozilla.org/mozilla-central/rev/3386b7f0af78 - mozilla42??
Could you please elaborate on "pre-existing".

Comment 48

[Jorg K (CEST = GMT+2)]

Can you tell by comparing:
https://hg.mozilla.org/releases/mozilla-esr38/file/tip/caps/nsPrincipal.cpp
https://hg.mozilla.org/releases/mozilla-esr45/file/tip/caps/nsPrincipal.cpp
https://hg.mozilla.org/releases/mozilla-esr52/file/tip/caps/nsPrincipal.cpp

What would the origin have been in those versions for, say:
mailbox:///path/to/mailbox/file?number=55 or
imap://user@domain@server:port/fetch>UID>folder>nn

Comment 49

[Gerry Chang [:gchang]]

Comment on attachment 8856364 [details] [diff] [review]
aurora.patch: Part 1 for M-A and M-B and M-esr52

For Thunderbird only. Aurora54+.

Comment 50

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6a87495d2630

Comment 51

[Boris Zbarsky [:bzbarsky]]

> Sorry, I can't name the regressing bug

"Whatever started using the return value of nsIPrincipal::GetOrigin for something someone cares about".

> Without this patch for TB, which spec would be used?

Without this patch, nsIPrincipal::GetOrigin returns the scheme+host+port of the mail server for imap:// URLs, say.  This means that anything that actually uses that return value will think all mails are same-origin.

> If not the full spec, when did that change, so what is the regressing bug?

It's never really changed.  What changes is that we used to not do anything with nsIPrincipal::GetOrigin other than some plug-in edge case stuff, but then people started using it for other things and thinking it was reliable...

> I also need to know in order to uplift this accordingly.

You should probably uplift to all supported branches.  That's simpler than trying to figure out who is using GetOrigin and when they started using it.... and the answer probably ends up being the same in the end.

Comment 52

[Jorg K (CEST = GMT+2)]

Comment on attachment 8856364 [details] [diff] [review]
aurora.patch: Part 1 for M-A and M-B and M-esr52

OK, since I got M-A approval, let's go for M-ESR52 approval. As I said, the hunk is only compiled for Thunderbird, so no harm done to FF.

Comment 53

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #38)
> (In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #35)
> > God, I wish Thunderbird actually had tests for its security stuff.  :(
> We do.e.g.
> https://dxr.mozilla.org/comm-central/source/mail/test/mozmill/content-policy,
> but maybe not as extensive as we should.
Filed bug 1355749 for more tests. Boris, could you please look over the description there and tell me whether I got it right.

And, BTW, many thanks for you continued support and comment #51. We'll take you up on your offer for a "training session". I'll organise via PM.

Comment 54

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8856364 [details] [diff] [review]
aurora.patch: Part 1 for M-A and M-B and M-esr52

This is needed for Thunderbird, NPOTB for Firefox.

Comment 55

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/b5a21502aeff