Bug 1246956 (CVE-2016-1967)

Stealing of URL cross-domain using performance.getEntries() after restore previous session

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jordi.chancel, Assigned: dragana)

Tracking

({csectype-sop, regression, sec-high})

44 Branch
Firefox 47
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
qe-verify -

Firefox Tracking Flags

(firefox45+ fixed, firefox46+ fixed, firefox47+ fixed, firefox-esr3846+ fixed, firefox-esr45- fixed)

Details

(Whiteboard: [adv-main45+][post-critsmash-triage][adv-esr38.8+])

Attachments

(7 attachments, 7 obsolete attachments)

748 bytes, text/html
Details
2.83 KB, application/zip
Details
749 bytes, text/html
Details
3.27 KB, patch
bzbarsky
: review+
Yoric
: review+
mfinkle
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
3.09 KB, patch
Yoric
: review+
mfinkle
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
4.59 KB, patch
Details | Diff | Splinter Review
3.12 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160123151951

Steps to reproduce:

It is possible to read cross-origin URLs following a redirect if perfomance.getEntries() is used along with an iframe to host a page after a first loading of this malicious webpage and the closing of Firefox (*) and restored the previous session. 

using the JavaScript function "history.back()" , the closing of Firefox and restored the previous session, content is pulled from the browser cache for the redirected location instead of going to the original location. 

* ( A Crash bug can be used to force Firefox to close without user interaction )


(I will upload a demonstration video to show you that)


Actual results:

Cross Origin URLs is stolen , this is a same-origin policy violation and could allow for data theft.



Expected results:

The stealing of a Cross Origin URL shouldn't be possible normally but this vulnerability demonstrates that it is posssible using these steps.
(Reporter)

Comment 1

3 years ago
Demonstration video.
Component: General → Document Navigation
(Reporter)

Comment 2

3 years ago
This is the testcase.
(Reporter)

Comment 3

3 years ago
You should download this testcase and open it into a new tab without previous webpage because this testcase uses history.back() and it redirects you on the previous page if it is directly loaded on the same webpage than the page which contains the link to go on it.
(Reporter)

Comment 4

3 years ago
Now you can download and use this new testcase , index.html contains a link which will open the Testcase into a new tab (without previous webpage) , after the loading of the testcase , you should close and reopen Firefox and restore the previous session and the testcase will works (only tested on Mac OS X El Capitan 10.11).
Attachment #8717557 - Attachment is obsolete: true
(Reporter)

Comment 5

3 years ago
I've found others steps .

1) Open the PoC and close the firefox window (with all tabs)
2) into the History menu , on the selection : windows recently opened , reopen the window closed which has contained the testcase .

Result:

The Cross-Origin URL with its "token key" is stolen.

(I will again upload a demonstration video to show you these others steps)
Summary: Stealing of URL cross-domain using perfomance.getEntries() after restore previous session → Stealing of URL cross-domain using performance.getEntries() after restore previous session
Could you look at this, Olli? Thanks.
Flags: needinfo?(bugs)
So if this requires one to restore the session, this isn't a DocumentNavigation bug but session restore.
And so far I haven't managed to reproduce the issue, so it isn't quite clear to me what the issue is yet.
I do see the following though.
Error: Permission denied to access property "history"
oh, http://demo.vwzq.net/php/token_redirect.php isn't any real web thingie.
Flags: needinfo?(bugs)
Or it is down.

Jordi, is your server down or is the testcase missing some file which one should run on a local server or what?
Flags: needinfo?(jordi.chancel)
(Reporter)

Comment 11

3 years ago
http://demo.vwzq.net/php/token_redirect.php isn't on my server , it is the php file used in bug1185256 -> https://bugzilla.mozilla.org/show_bug.cgi?id=1185256

http://demo.vwzq.net/php/token_redirect.php generates a token like this : http://demo.vwzq.net/php/token_redirect.php?secret=c572909e1fa95a3bc28584cdd6ae341e

and now the server seems work.
Flags: needinfo?(jordi.chancel)
Can't reach that server here :/
(Reporter)

Comment 13

3 years ago
it works for me , please delete your caches and history and retryto open http://demo.vwzq.net/php/token_redirect.php , normally it will works
Flags: needinfo?(bugs)
Hmm, bug 1185256 added originalURI to nsISHEntry but didn't update any of session store code.
How is that code supposed to work? When should one use URI from nsISHEntry, when originalURI
Flags: needinfo?(dd.mozilla)
And can't reach http://demo.vwzq.net/php/token_redirect.php here.
I can't even ping that server. And all the browsers just tell the site doesn't exist.
Flags: needinfo?(bugs)
Anyhow, this looks like a variant of bug 1185256.
(Reporter)

Comment 17

3 years ago
others steps is possible to exploit this vulnerability, read comment5 -> https://bugzilla.mozilla.org/show_bug.cgi?id=1246956#c5
(Reporter)

Comment 18

3 years ago
Sometime the server is down , so , try sometime until the domain is accessible.
> Anyhow, this looks like a variant of bug 1185256.

Yes.  Sessionstore code should probably store both bits of state and restore them both...
Group: firefox-core-security
Component: Document Navigation → Session Restore
Product: Core → Firefox
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 20

3 years ago
I will take this.
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(dd.mozilla)
(Reporter)

Comment 22

3 years ago
The PoC works on :

Firefox stable (44.0.1), Firefox Beta (45) and Nightly (47.0a1).
(In reply to Olli Pettay [:smaug] (high review load) from comment #16)
> Anyhow, this looks like a variant of bug 1185256.

I'll mark this sec-high, too, then. Thanks everybody for looking at this.
Flags: sec-bounty?
(Assignee)

Comment 24

3 years ago
Attachment #8719591 - Flags: review?(bzbarsky)
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

r=me, assuming you've verified this fixes the bug, but it's a good idea to also get review from the actual owners of the code being changed...

Also, adding a test, if possible, would be awesome.
Attachment #8719591 - Flags: review?(bzbarsky) → review+
Marking affected and tracking for 45 and 47.  
Jordi did you try this on 46? 
Dragana if this applies to 46 please request uplift to aurora (and sounds like to beta as well)
How about ESR 38?
Flags: needinfo?(jordi.chancel)
Flags: needinfo?(dd.mozilla)
(Reporter)

Comment 27

3 years ago
Have you got the download URL for the 46 version (testing version) or another testing version than I must try/verify, please?
Flags: needinfo?(lhenry)
(Assignee)

Comment 29

3 years ago
Comment # 28 is a try run. You can test it using that build.
It is going to be in nightly  soon.
Flags: needinfo?(lhenry)
(Reporter)

Comment 30

3 years ago
NightlyDebug 47.0a1 (2016-02-18) ( http://archive.mozilla.org/pub/firefox/try-builds/dd.mozilla@gmail.com-4ce4928d02e0d275d49e3a6c2f853699e537ab85/try-macosx64-debug/firefox-47.0a1.en-US.mac64.dmg ) seems to be fixed.

Dragana Damjanovic, can you send me a needinfo flag request when the Nightly build is ready, please?

Thanks
Flags: needinfo?(jordi.chancel)
(Assignee)

Comment 31

3 years ago
(In reply to Jordi Chancel from comment #30)
> NightlyDebug 47.0a1 (2016-02-18) (
> http://archive.mozilla.org/pub/firefox/try-builds/dd.mozilla@gmail.com-
> 4ce4928d02e0d275d49e3a6c2f853699e537ab85/try-macosx64-debug/firefox-47.0a1.
> en-US.mac64.dmg ) seems to be fixed.
> 
> Dragana Damjanovic, can you send me a needinfo flag request when the Nightly
> build is ready, please?
> 
> Thanks

I will needinfo you when this patch is in nightly.
(Assignee)

Comment 32

3 years ago
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

Please can you take a look at the patch. Thanks!
Attachment #8719591 - Flags: review?(mark.finkle)
Attachment #8719591 - Flags: review?(dteller)
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

Review of attachment 8719591 [details] [diff] [review]:
-----------------------------------------------------------------

I can't confirm that it fixes the issue, but the patch makes sense.
Can't review the Android version.

For bonus points, could you explain the issue and fix in the commit message?
Attachment #8719591 - Flags: review?(dteller) → review+
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

Thanks for the fix!
Attachment #8719591 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 35

3 years ago
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
This is a long existing, since ff 31, security vulnerability that was partially fixed by bug 1185256. That bug did not fix one scenario involving browser history.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Bug 1185256 was publicly know and considering the change made for that bug and change make in this bug connection can be made.

Which older supported branches are affected by this flaw?
since ff 31
If not all supported branches, which bug introduced the flaw?
Bug 822480

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I think the same patch will applied to all. This code have not changed much.

How likely is this patch to cause regressions; how much testing does it need?
Not very likely. It only adds additional parameter to  session store, should not need much testing.
Attachment #8719591 - Flags: sec-approval?
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

sec-approval+ for trunk.

We'll want this on Aurora, Beta, and ESR38. Please create and nominate patches for those so that they can land after it is on trunk.
Attachment #8719591 - Flags: sec-approval? → sec-approval+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Blocks: 822480
Group: firefox-core-security, core-security → core-security-release
Keywords: regression
https://hg.mozilla.org/mozilla-central/rev/56ed0ec901c0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Dragana, could you please nominate patches for esr38, beta45 and aurora46? Thanks!
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 40

3 years ago
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

Approval Request Comment
[Feature/regressing bug #]:Bug 822480
[User impact if declined]: It is a security issue - stealing of URL cross-domain using performance.getEntries() when a resource is loaded from history.
[Describe test coverage new/current, TreeHerder]: Steps to reproduce exist.
[Risks and why]: Low it adds one parameter to the session history.
[String/UUID change made/needed]: none
Flags: needinfo?(dd.mozilla)
Attachment #8719591 - Flags: approval-mozilla-beta?
Attachment #8719591 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 41

3 years ago
This patch is dependent on patch from bug 1185256. Bug 1185256 never landed on esr38.
(In reply to Dragana Damjanovic [:dragana] from comment #41)
> This patch is dependent on patch from bug 1185256. Bug 1185256 never landed
> on esr38.

Thanks Dragana. I'll get the patch from that bug uplifted to esr38. Could you please nominate this one for uplift in the meantime?
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 43

3 years ago
Posted patch bug_1246956_esr38.patch (obsolete) — Splinter Review
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version: 47
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(dd.mozilla)
Attachment #8723475 - Flags: approval-mozilla-esr38?
Comment on attachment 8719591 [details] [diff] [review]
bug_1246956.patch

Should be safe, taking it.
Should be in 45 beta 10
Attachment #8719591 - Flags: approval-mozilla-beta?
Attachment #8719591 - Flags: approval-mozilla-beta+
Attachment #8719591 - Flags: approval-mozilla-aurora?
Attachment #8719591 - Flags: approval-mozilla-aurora+
Comment on attachment 8723475 [details] [diff] [review]
bug_1246956_esr38.patch

Sec-high issues meet esr uplift bar.
Attachment #8723475 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Dan, Al: Dragana mentioned that we also need to uplift the fix from bug 1185256 in order to address this sec vulnerability. However, the fix from that bug has a UUID change that could potentially break addons and/or cause crashes. Please see: https://bugzilla.mozilla.org/show_bug.cgi?id=1185256#c51

Given that, I am leaning towards wontfix'ing this bug for esr38.7. Any concerns?
Flags: needinfo?(dveditz)
Flags: needinfo?(abillings)
Yes, I'm concerned.

Not taking the fix for bug 1185256 is the original problem. I agree w/Jorge that the patch as taken on the trunk has a chance of breaking add-ons: I found ~10 using nsIDocShellLoadInfo and ~150 using nsISHEntry. Those were all JS, of course, and therefore not a problem, but that's enough usage that it's reasonable to worry about binary add-ons using these APIs.

But taking the simplistic merge of the m-c patch onto the ESR isn't the only way to do it, in fact it was routinely _not_ the way to do it. What should happen is that you leave the original interfaces alone and create
  interface nsISHEntry2 : nsISHEntry
  interface nsIDocShellLoadInfo2 : nsIDocShellLoadInfo

  (do we need to do nsIDocShell? if a binary add-on is trying to use that
  won't they break pretty regularly? Maybe we should, though.)

with the new members. In the places where we need this new data you QI to the new interface, otherwise everything works as before. It's pretty straightforward, just some extra boilerplate. That was, and should still be, the standard way of dealing with interface changes on long-lived branches. We can't just not-take security fixes.
Flags: needinfo?(dveditz)
Patches reworked in this way go beyond simple merges and should get an independent re-review. It's usually a light/simple re-review, but not appropriate to just carry over the original review.
(Assignee)

Comment 51

3 years ago
I could rework the patch according to comment 49, if the decision is to uplift.
(In reply to Dragana Damjanovic [:dragana] from comment #51)
> I could rework the patch according to comment 49, if the decision is to
> uplift.

Yes, I will definitely take a patch that does not break add-on compatibility or stability and still addresses this sec-high issue in esr38.7. Please nominate the patch for uplift once ready and reviewed. Thanks!
Flags: needinfo?(abillings)

Updated

3 years ago
Depends on: 1251754
(Assignee)

Comment 53

3 years ago
Sorry, I forgot one parameter. With out this parameter some pages can be strange.
Attachment #8724284 - Flags: review?(mark.finkle)
Attachment #8724284 - Flags: review?(dteller)
(Assignee)

Comment 54

3 years ago
Posted patch bug_1246956_esr38.patch (obsolete) — Splinter Review
Attachment #8723475 - Attachment is obsolete: true
Attachment #8724284 - Flags: review?(mark.finkle) → review+
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

Review of attachment 8724284 [details] [diff] [review]:
-----------------------------------------------------------------

Apologies for the delay, apparently security bugs don't show up on my dashboard.
Attachment #8724284 - Flags: review?(dteller) → review+
(Assignee)

Comment 56

3 years ago
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

Sorry for asking approval again. I have forgot one parameter that needs to be stored as well. I forgot that in the first patch, so it causes some pages not to be loaded correctly. This patch does not solve or cause security issue, it just fixes a problem caused by the first patch.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Which older supported branches are affected by this flaw?

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

How likely is this patch to cause regressions; how much testing does it need?
Attachment #8724284 - Flags: sec-approval?
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

sec-approval+ for trunk but you need to nominate this ASAP for aurora and beta. We're supposed to go to build within the next 24 hours and this needs to be in the build.

Also, why is there no ESR38 patch for this if ESR38 is affected?
Attachment #8724284 - Flags: sec-approval? → sec-approval+
Flags: needinfo?(dd.mozilla)
Whiteboard: [adv-main45+]
(In reply to Al Billings [:abillings] from comment #57)
> Comment on attachment 8724284 [details] [diff] [review]
> bug_1246956_part2.patch
> 
> sec-approval+ for trunk but you need to nominate this ASAP for aurora and
> beta. We're supposed to go to build within the next 24 hours and this needs
> to be in the build.
> 
> Also, why is there no ESR38 patch for this if ESR38 is affected?

I could be wrong but I think this patch is ESR38 only.
If that's the case, then this patch should be asking for ESR38 approval, not sec-approval. Sec-approval is for trunk only.
(Assignee)

Comment 60

3 years ago
this is not for ESR38.
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 61

3 years ago
Check in needed only for patch: bug_1246956_part2.patch 

The other one is already in the repo.
Keywords: checkin-needed
(Assignee)

Comment 62

3 years ago
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

Approval Request Comment
[Feature/regressing bug #]: This is a missing part that should have been in the first patch. It does not solves the security issue. The current 45 and 46 will show some pages wrong and this patch fixes that.
[User impact if declined]: e.g. bug 1251754
[Describe test coverage new/current, TreeHerder]: One of the way this regression can be seen is bug 1251754. I have retested with this patch.
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8724284 - Flags: approval-mozilla-beta?
Attachment #8724284 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 63

3 years ago
For ESR38 we need to uplift bugs 1185256, 1211269 and 1213267. They need a new review because they are changing uuids and there are patches waiting to be reviewed. After that we can uplift this one. This one will not need a new review it changes only js code.
Alias: CVE-2016-1967
(In reply to Dragana Damjanovic [:dragana] from comment #61)
> Check in needed only for patch: bug_1246956_part2.patch 
> 
> The other one is already in the repo.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dcc4b8b947d
I tried to reproduce the initial issue, but without success. I've downloaded the testcase and I've opened index.html; the Testcase is opened in a new tab (without previous webpage). After that, Ive followed 2 scenarios: 
- I've closed and reopened Firefox and restored the previous session (from about:home and from History menu) -> the issue does not reproduce
- I've closed the Firefox window (with all tabs) and from History menu, I've selected "Recently closed windows"; the window with the testcase is reopened, but the issue is not reproduced. 

The "token key" is not displayed, only "Page http://demo.vwzq.net/php/token_redirect.php redirected to .... " is shown. 

Have I missed something? 

I've tried on Mac OSX 10.11.1 and Windows 7 64bit using Firefox 44 (buildID: 20160123151951), Firefox 44.0.1 and Firefox 45 Beta 2.
Flags: needinfo?(jordi.chancel)

Updated

3 years ago
Depends on: 1252629
(In reply to Carsten Book [:Tomcat] from comment #64)
> (In reply to Dragana Damjanovic [:dragana] from comment #61)
> > Check in needed only for patch: bug_1246956_part2.patch 
> > 
> > The other one is already in the repo.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3dcc4b8b947d

Does this mean the patch currently in Firefox 45 is not complete?
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 67

3 years ago
Patch bug_1246956.patch is in 45, 46, 47
And patch bug_1246956_part2.patch is not in 45 and 46 (it is just pushed in 47).

We need the second patch in 45  and 46 as well, without it we have bugs like: 1252629 and 1251754.

Sorry for this, I forgot to add one parameter and just remembered that we need that list Friday, but the new patch reviews took some time.
Flags: needinfo?(dd.mozilla)
I'm going to assume we aren't fixing this for ESR38.7, being built any moment now as well.
(Assignee)

Comment 69

3 years ago
(In reply to Al Billings [:abillings] from comment #68)
> I'm going to assume we aren't fixing this for ESR38.7, being built any
> moment now as well.

it is blocked on bugs 1185256, 1211269 and 1213267
I just noticed that Bz wants one of the patches fixed before uplift. See: https://bugzilla.mozilla.org/show_bug.cgi?id=1185256#c54. Given that and the timing of esr38.7-build1 gtb, we will need to wontfix this for esr38.7 and land them in esr38.8. Just wanted to let everyone know.
(Assignee)

Comment 71

3 years ago
Posted patch bug_1246956_esr38_v1.patch (obsolete) — Splinter Review
Just rebased.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8725834 - Flags: approval-mozilla-esr38?
(Assignee)

Comment 72

3 years ago
Just rebased.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8724429 - Attachment is obsolete: true
Attachment #8725835 - Flags: approval-mozilla-esr38?
(Assignee)

Comment 73

3 years ago
Posted patch bug_1246956_esr38_v1.patch (obsolete) — Splinter Review
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8725834 - Attachment is obsolete: true
Attachment #8725834 - Flags: approval-mozilla-esr38?
Attachment #8725911 - Flags: approval-mozilla-esr38?
(Assignee)

Comment 74

3 years ago
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8725835 - Attachment is obsolete: true
Attachment #8725835 - Flags: approval-mozilla-esr38?
Attachment #8725912 - Flags: approval-mozilla-esr38?
(Assignee)

Comment 75

3 years ago
sorry, typo

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8725911 - Attachment is obsolete: true
Attachment #8725911 - Flags: approval-mozilla-esr38?
Attachment #8725913 - Flags: approval-mozilla-esr38?
(Assignee)

Comment 76

3 years ago
rebased.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8725912 - Attachment is obsolete: true
Attachment #8725912 - Flags: approval-mozilla-esr38?
Attachment #8725916 - Flags: approval-mozilla-esr38?
Comment on attachment 8725913 [details] [diff] [review]
bug_1246956_esr38_v1.patch

This is also needed in esr38.7
Attachment #8725913 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Attachment #8725916 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
(Reporter)

Comment 78

3 years ago
I've tried on Firefox 44.0.2 on Mac OS X 10.11 El Captain and the proof of concept works , but sometime the server "http://demo.vwzq.net/" is not accessible (server down) , if this website is down , we can not reproduce the issue. 

(And like demonstrated in the video example we must close firefox or the tab in a specific time before restart firefox or reload the tab closed for that the testcase works as showed in the video )

But this testcase has already been reproduced by others developpers when the website "http://demo.vwzq.net/" is accessible.

I tried this vulnerability on the last Firefox Beta version (45) and I seen that it seems perfectly resolved / fixed .
Flags: needinfo?(jordi.chancel)

Updated

3 years ago
Flags: sec-bounty? → sec-bounty+
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

Not needed if I understood correctly
Attachment #8724284 - Flags: approval-mozilla-beta?
Attachment #8724284 - Flags: approval-mozilla-beta-
Attachment #8724284 - Flags: approval-mozilla-aurora?
Attachment #8724284 - Flags: approval-mozilla-aurora-
(Assignee)

Comment 82

3 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #81)
> Comment on attachment 8724284 [details] [diff] [review]
> bug_1246956_part2.patch
> 
> Not needed if I understood correctly

We really need this. This fixes bugs like 1252629 and 1251754 which are in beta and aurora.
Comment on attachment 8724284 [details] [diff] [review]
bug_1246956_part2.patch

Looks like we have to take it
Attachment #8724284 - Flags: approval-mozilla-release+
Attachment #8724284 - Flags: approval-mozilla-esr45+
Attachment #8724284 - Flags: approval-mozilla-beta-
Attachment #8724284 - Flags: approval-mozilla-beta+
Attachment #8724284 - Flags: approval-mozilla-aurora-
Attachment #8724284 - Flags: approval-mozilla-aurora+
[Tracking Requested - why for this release]: ESR45 is branched, so we should probably track this separately for it as well.

https://hg.mozilla.org/releases/mozilla-beta/rev/a39cbdebde6c
https://hg.mozilla.org/releases/mozilla-release/rev/a39cbdebde6c
https://hg.mozilla.org/releases/mozilla-esr45/rev/a39cbdebde6c

Setting status-firefox46 back to affected so this shows up on the Aurora uplift radar.

Comment 85

3 years ago
Tested on Firefox for Android. 
Following the STR from the test case in this bug, I wasn't able to reproduce this issue on an affected build.
I've tried all the scenarios, and also checked that "http://demo.vwzq.net/php/token_redirect.php" is accessible. 
Can someone reproduce this on an affected build on Android, so this can be verified as fixed on Firefox for Android 45 RC build 3?
(In reply to Jordi Chancel from comment #78)

> I tried this vulnerability on the last Firefox Beta version (45) and I seen
> that it seems perfectly resolved / fixed .

Marking qe-verify- based on this.
Flags: qe-verify-
Whiteboard: [adv-main45+] → [adv-main45+][post-critsmash-triage]
Whiteboard: [adv-main45+][post-critsmash-triage] → [adv-main45+][post-critsmash-triage][adv-esr38.8+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.