Bug 1254688 (CVE-2016-5250)

Resource Timing API is storing resources sent by the previous page.

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: catalind, Assigned: valentin, NeedInfo)

Tracking

(Blocks: 1 bug, {csectype-sop, privacy, sec-moderate})

45 Branch
mozilla49
csectype-sop, privacy, sec-moderate
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox46 wontfix, firefox47- wontfix, firefox48+ fixed, firefox49+ fixed, firefox-esr4549+ fixed)

Details

(Whiteboard: [land tests in July][adv-main48+][adv-esr45.4+] maybe sec-high? btpp-active)

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
strtestcase
Created attachment 8728078 [details]
Test pages which reproduce the error

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.116 Safari/537.36

Steps to reproduce:

We've identified an issue with the Resource Timing API, in scenarios in which requests are sent during a navigation from one page to another. Requests sent by the previous page, which were made after the next page starts loading, will appear in the list of resources for the next page. This typically happens if requests are sent during the "onbeforeunload" and "unload" handlers. These resources will appear at the beginning of the array returned by calling:

    performance.getEntriesByType("resource")

This scenario also reproduces for cross-origin navigations, which is why this can pose a security and privacy issue, considering that any page can detect and capture some of the requests sent by the previous page.


Steps to Reproduce:

Attached are two files used as test pages to reproduce this bug. Each page will attach a handler to the "unload" event which will perform an external request with an identifier for the current page. This request has the format:

http://www.google-analytics.com/r/collect?dl={{current page}}

Additionally, when the page initially loads, with a 2 delay, a list of resources for the current page will be populated in the "Current list of resources" section, extracted by calling "performance.getEntriesByType('resource')".

1. Ensure any "AdBlock" extensions are disabled before opening these test pages. They might block the external request.​
2. Open "page1.html" in Firefox
3. Wait 2 seconds for the list of resources to be displayed. The initial list should be empty as the first page hasn't made any requests yet.
4. Press the bottom link to navigate to the next page.
5. Wait 2 seconds for the new list of resources to be displayed. This time, we see the list consists of a single request to "http://www.google-analytics.com" sent by the previous page.


Actual results:

The list of resources should be empty on both accounts, for page1 and for page2, as the requests are made after the initial 2 seconds delay, during the "unload" event.


Expected results:

On page2, there is a single request made by page1. In essence, page2 has captured a request made by page1.

We've tested this scenario on Chrome and IE11, and in both cases, the lists of resources are empty.

Updated

3 years ago
Component: Untriaged → DOM
Product: Firefox → Core
(Reporter)

Comment 1

3 years ago
I seem to have made a mistake in the description of the bug, the "Actual results" and "Expected results" sections are reversed. It should read:

Actual results:

On page2, there is a single request made by page1. In essence, page2 has captured a request made by page1.


Expected results:

The list of resources should be empty on both accounts, for page1 and for page2, as the requests are made after the initial 2 seconds delay, during the "unload" event.
Valentin, what do you think?
Group: dom-core-security
Flags: needinfo?(valentin.gosu)
Attachment #8728078 - Attachment mime type: application/zip → application/java-archive
Confirmed on nightly.
Blocks: 822480
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: csectype-sop, privacy, sec-moderate
Whiteboard: maybe sec-high?
(Assignee)

Comment 4

3 years ago
Thanks for the report.
I suspect the docshell's performance object is switched in onunload, and we end up saving the resources in the wrong array.
Assignee: nobody → valentin.gosu
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 5

3 years ago
So, the problem here is that during the onunload handler we initialize the object which creates a channel and calls :AsyncOpen.
When the request completes, in :OnStopRequest we call HttpBaseChannel::GetPerfomance() which goes through the loadContext -> domWindow -> nsPerformance. At this point unload has already finished, and the docshell has changed the window for a new one.
I've tried caching the performance object in :AsyncOpen, to make sure we're using the correct one. That seems to work but it fails a mochitest for iframe behaviour - currently debugging.
We should be getting to the performance object via the loadinfo, I would think.  That represents the actual Document and Window the load is associated with...
(Assignee)

Comment 7

3 years ago
(In reply to Boris Zbarsky [:bz] from comment #6)
> We should be getting to the performance object via the loadinfo, I would
> think.  That represents the actual Document and Window the load is
> associated with...

Getting the performance object via mLoadInfo->GetLoadingDocument -> mozIDOMWindowProxy::GetDefaultView ... innerWindow->GetPerformance() seems to have the same problem.
If you're going through mozIDOMWindowProxy you have clearly lost, since that's the outer window.  You want to go through nsIDocument::GetInnerWindow().
(Assignee)

Comment 9

3 years ago
Created attachment 8737899 [details] [diff] [review]
Use mLoadInfo to get the performance object

Thanks for the hint, Boris. That worked, except I get a null pointer for GetParentPerformance() in regular iframe use cases. Do you know why there would be a difference between the 2 ways of getting the performance object?
Attachment #8737899 - Flags: feedback?(bzbarsky)
Which two ways are you talking about?
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 11

3 years ago
(In reply to Boris Zbarsky [:bz] from comment #10)
> Which two ways are you talking about?

The one we currently use: 
> loadContext->GetAssociatedWindow(); pDomWindow->GetCurrentInnerWindow();
>     innerWindow->GetPerformance(); docPerformance->GetParentPerformance();
vs the one you suggested:
> mLoadInfo->GetLoadingDocument(); loadingDocument->GetInnerWindow();
> innerWindow->GetPerformance(); docPerformance->GetParentPerformance();

While writing the comment I actually realized that there's no need for docPerformance->GetParentPerformance() when using mLoadInfo to get the document, but I'm still not quite sure why that is.
Flags: needinfo?(valentin.gosu)
> loadContext->GetAssociatedWindow(); pDomWindow->GetCurrentInnerWindow();
>     innerWindow->GetPerformance(); docPerformance->GetParentPerformance();

OK, so this gets the corresponding window for the load context (the docshell), then gets its performance object, then does GetParentPerformance.  This makes sense for document loads, because the loadcontext for a document load is the subframe docshell, so the performance object you would get is for that subframe's window, but you want to put it in the parent.  For non-document loads the performance object off the loadcontext is the right one, which is why the GetParentPerformance is done only when LOAD_DOCUMENT_URI.

> mLoadInfo->GetLoadingDocument(); loadingDocument->GetInnerWindow();
> innerWindow->GetPerformance();

The loading document is defined as the document the load got started from.  Its semantics for toplevel loads are a bit unclear (please check with tanvi as to what they are nowadays); for subframe loads it's the document containing the <iframe> or <frame> tag.  For non-document loads it's the document the load is associated with.  So in this case the performance object you get is already the one for the parent document, in the case of subframe loads.

Note that the semantics of loadingDocument are documented in nsILoadInfo.idl; if that documentation is not clear pleas file bugs.
Whiteboard: maybe sec-high? → maybe sec-high? btpp-active
(Assignee)

Comment 13

3 years ago
Created attachment 8737916 [details] [diff] [review]
Use mLoadInfo to get the performance object

MozReview-Commit-ID: KMbwR7J8FLm
Attachment #8737916 - Flags: review?(bzbarsky)
(Assignee)

Updated

3 years ago
Attachment #8737899 - Attachment is obsolete: true
Attachment #8737899 - Flags: feedback?(bzbarsky)
(Assignee)

Comment 14

3 years ago
Created attachment 8737958 [details] [diff] [review]
Test - Resources loaded in onunload shouldn't show up in the next page performance object

MozReview-Commit-ID: EI1daHa9VTp
Attachment #8737958 - Flags: review?(bzbarsky)
Comment on attachment 8737916 [details] [diff] [review]
Use mLoadInfo to get the performance object

>+  nsCOMPtr<nsIDOMDocument> domDocument;
>+  mLoadInfo->GetLoadingDocument(getter_AddRefs(domDocument));

I think it might be a bit simpler to do:

  nsINode* loadingNode = mLoadInfo->LoadingNode();
  if (!loadingNode) {
    return nullptr;
  }

  nsPIDOMWindowInner* innerWindow = loadingNode->OwnerDoc()->GetInnerWindow();

Though the fact that LoadingNode() can return null is weird; worth filing a bug on that: it should have a Get prefix if it can return null.

>+  if (!docPerformance) {
>+    return nullptr;
>+  }
>+
>+  return docPerformance;

Why do you need that if?  Just return docPerformance; if it's null, it's null.

This seems reasonable to me, but please double-check with Tanvi as to whether the LoadingNode() of mLoadInfo is reliable enough at this point, and whether it's that way on the branches you plan to backport this to....
Attachment #8737916 - Flags: review?(tanvi)
Attachment #8737916 - Flags: review?(bzbarsky)
Attachment #8737916 - Flags: review+
Comment on attachment 8737958 [details] [diff] [review]
Test - Resources loaded in onunload shouldn't show up in the next page performance object

>+  while (performance.now() - start < 1000);

What's the point of this line?  I don't expect it to have any effect whatsoever....

r=me with that removed or clearly explained...
Attachment #8737958 - Flags: review?(bzbarsky) → review+

Comment 17

3 years ago
(In reply to Boris Zbarsky [:bz] from comment #15) 
> This seems reasonable to me, but please double-check with Tanvi as to
> whether the LoadingNode() of mLoadInfo is reliable enough at this point, and
> whether it's that way on the branches you plan to backport this to....

What content types is GetPerformance() called for?  TYPE_DOCUMENT loads don't have an accurate loadingNode and soon won't have one at all, but from Comment 0 it looks like this is only an issue for subresources.  Does it apply to TYPE_SUBDOCUMENT?

loadingPrincipal is required for non-TYPE_DOCUMENT loads, but loadingNode is not.  There are cases where we have a principal, but no node.  And specifically in e10s, we don't always have a node.

Valentin, I'm happy to chat more about this on irc or vidyo if you would like.  I'm out tomorrow but will be back online on Wednesday.  Christoph is also out right now, but you may be able to find sicking on irc.
> What content types is GetPerformance() called for?

I'm guessing all of them, though for TYPE_DOCUMENT we really want to end up with null.

> Does it apply to TYPE_SUBDOCUMENT?

Yes.

> There are cases where we have a principal, but no node. 

What cases are those?

Comment 19

3 years ago
loadInfo->loadingNode is not reliable for TYPE_DOCUMENT.
** In e10s mode it is often wrong before Bug 1113196/Firefox 47.
** In e10s mode it is null after Bug 1113196/Firefox 47
** With Bug 1105556 (probably Firefox 49), it will be null for non-e10s mode and e10s mode.

loadInfo->loadingNode and loadInfo->loadingPrincipal should be reliable for TYPE_SUBDOCUMENT loads after Bug 1113196 and bug 1240246 in Firefox 47.  (It may also be reliable before Firefox 47, but I can't be sure without better understanding the changes in bug 1240246.  There may be circumstances where we couldn't get a node and couldn't get a principal, and fell back to not-necessarily-sane values.)

(In reply to Boris Zbarsky [:bz] from comment #18)
> > There are cases where we have a principal, but no node. 
> 
> What cases are those?

Here are a some examples, along with some random places in the code I found where we don't have a loadingNode.  There are likely many more.

* When we go from e10s child to parent or parent to child: http://mxr.mozilla.org/mozilla-central/source/ipc/glue/BackgroundUtils.cpp#207.  Example: https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp#93

* Per nsNetUtil.h, For loads that are not related to any document, such as loads coming from addons or internal browser features.  And WebWorkers

* calls to NS_NewInputStreamChannel() https://mxr.mozilla.org/mozilla-central/ident?i=NS_NewInputStreamChannel

* in nsNetUtil, we have multiple NewChannel methods to account for cases where we have a
  i)   loadingNode, loadingPrincipal and triggeringPrincipal
  ii)  loadingNode, loadingPrincipal
  iii) loadingPrincipal, triggeringPrincipal
  iv)  just a loadingPrincipal
Callers choose which method to call based on what parameters they have available.  I don't remember all the cases where we call the methods for iii or iv[1], but perhaps Christoph or Jonas can think of a few good examples.

* http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsFaviconService.cpp#350

* https://mxr.mozilla.org/mozilla-central/source/image/imgLoader.cpp#782

[1] https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#212
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#237
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#299
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#328
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#410

Comment 20

3 years ago
(In reply to Boris Zbarsky [:bz] from comment #12)
> > loadContext->GetAssociatedWindow(); pDomWindow->GetCurrentInnerWindow();
> >     innerWindow->GetPerformance(); docPerformance->GetParentPerformance();
> 
> ... For non-document loads the performance object off the loadcontext is the right
> one, which is why the GetParentPerformance is done only when
> LOAD_DOCUMENT_URI.
> 

If the problem with the existing code that uses loadcontext is only for subframes then maybe we can:
* use loadcontext for non-TYPE_DOCUMENT, non-TYPE_SUBDOCUMENT loads
* Return null for TYPE_DOCUMENT loads
* use loadinfo->LoadingNode for TYPE_SUBDOCUMENT loads

Comment 21

3 years ago
Comment on attachment 8737916 [details] [diff] [review]
Use mLoadInfo to get the performance object

r- for now, since we can't depend on the loadingDocument for all content types.
Attachment #8737916 - Flags: review?(tanvi) → review-
> If the problem with the existing code that uses loadcontext is only for subframes

It's not.  It's a problem for toplevel documents too.

We really need to fix things so that for anything that's not a TYPE_DOCUMENT load that's associated with a web document we properly track which document it's associated with; I thought LoadInfo was supposed to solve that problem.  Because there is no sane way to do something like resource timing unless we do that...
> loadInfo->loadingNode is not reliable for TYPE_DOCUMENT.

That's fine.  The code involved in this bug doesn't care about TYPE_DOCUMENT.

> When we go from e10s child to parent or parent to child:

Is that an issue for HTTP at all?  If not, it probably doesn't matter for purposes of this bug, since this bug only cares about loadinfo on HTTP channels.

> For loads that are not related to any document, such as loads coming from addons or internal browser
> features.

Not an issue, since those are not relevant to web-exposed resource timing.

> calls to NS_NewInputStreamChannel()

Not an issue, since this code is HTTP-channel-specific.

>* https://mxr.mozilla.org/mozilla-central/source/image/imgLoader.cpp#782

Seems ok; the "not loading inside a document" case means we don't want to track that stuff in resource timing.

Sounds like the issues are: 1) people using NewChannel without loadingNode when the load _is_ document-associated, assuming we have such cases and 2) TYPE_SUBDOCUMENT prior to Firefox 47.  The latter may require some thought if we want to backport this past Firefox 47, but could be worked around by explicitly walking to the frameElement of the window of the docshell the load is happening in to find the right document/window for the relevant Performance object.
(Assignee)

Comment 24

3 years ago
Boris, I'm looking into landing this. The patch applies and works on m-c and aurora, and also beta with minimal changes.
Do we need Tanvi to r+ this as well?
Flags: needinfo?(bzbarsky)
(Assignee)

Comment 25

3 years ago
Created attachment 8748584 [details] [diff] [review]
Test - Resources loaded in onunload shouldn't show up in the next page performance object

Updated test removing unnecessary while loop. Using daemon.png instead of red.png because bug 1113676 makes it pass even when it shouldn't.
(Assignee)

Updated

3 years ago
Attachment #8737958 - Attachment is obsolete: true
Yes, please talk to tanvi about the situation with loadingDocument on all the various branches and whether it does what we want it to do.  I haven't been keeping track of what's going on there and what all the edge cases are, so I simply don't know whether there are issues or not.
Flags: needinfo?(bzbarsky)
(Assignee)

Comment 27

3 years ago
Created attachment 8750426 [details] [diff] [review]
Use mLoadInfo to get the performance object

Updated with Tanvi's suggestions that we return early for TYPE_DOCUMENT, or if we're in the parent process on e10s
Attachment #8750426 - Flags: review?(tanvi)
(Assignee)

Updated

3 years ago
Attachment #8737916 - Attachment is obsolete: true

Comment 28

3 years ago
Discussed with Christoph and Jonas again today.  As mentioned in comment 19, the loadingDocument is not always available.  In most cases, that is because there is no document.  There may be cases, however, where there is a document involved but we just don't have it.  Determining all of these cases would involve a large audit, but we have compiled a list from memory:

favicons - favicon requests may happen in chrome code where we can't tell what document they belong to.  I'm not sure if this is for all favicon requests, or just some.  But it is worth testing.  This is not an easy problem to solve, so for now you may want to just accept that some favicon requests won't make it into the Performance object.
https://bugzilla.mozilla.org/show_bug.cgi?id=1119386, http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsFaviconService.cpp#415

Jonas suggested testing preloads.  He also suggested testing some image codepaths; in particular he said to check css background images and image preloads.  Finally, he said it is worth testing send beacon.
> so for now you may want to just accept that some favicon requests won't make it into the Performance object

They shouldn't make it in there at all, so that's fine.

Comment 30

3 years ago
Comment on attachment 8750426 [details] [diff] [review]
Use mLoadInfo to get the performance object

This patch looks fine.  Before pushing to inbound, I would do a MOZ_ASSERT(domDocument) here and push to try just to see what types of things might be failing:

>+  nsCOMPtr<nsIDOMDocument> domDocument;
>+  mLoadInfo->GetLoadingDocument(getter_AddRefs(domDocument));
>+  if (!domDocument) {
>+    return nullptr;
>+  }

Also, per below, there are a number of things you could test manually for correctness.  If you don't though, none of these issues are security issues.


(In reply to Tanvi Vyas out 5/16-5/17 [:tanvi] from comment #28)
> Discussed with Christoph and Jonas again today.  As mentioned in comment 19,
> the loadingDocument is not always available.  In most cases, that is because
> there is no document.  There may be cases, however, where there is a
> document involved but we just don't have it.  Determining all of these cases
> would involve a large audit, but we have compiled a list from memory:
> 
> favicons - favicon requests may happen in chrome code where we can't tell
> what document they belong to.  I'm not sure if this is for all favicon
> requests, or just some.  But it is worth testing.  This is not an easy
> problem to solve, so for now you may want to just accept that some favicon
> requests won't make it into the Performance object.
> https://bugzilla.mozilla.org/show_bug.cgi?id=1119386,
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/
> nsFaviconService.cpp#415
> 
> Jonas suggested testing preloads.  He also suggested testing some image
> codepaths; in particular he said to check css background images and image
> preloads.  Finally, he said it is worth testing send beacon.

Also, one more from Christoph - workers should have a loadingNode, but subworkers don't.
Attachment #8750426 - Flags: review?(tanvi) → review+
(Assignee)

Comment 31

2 years ago
Comment on attachment 8750426 [details] [diff] [review]
Use mLoadInfo to get the performance object

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

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
It is not immediately obvious what the problem is. The test in attachment 874858 does illustrate the vulnerability.

Which older supported branches are affected by this flaw?
All.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch can be easily backported to all branches.

How likely is this patch to cause regressions; how much testing does it need?
There is a slight risk of regressions that have been mentioned in previous comments. I have manually tested some of them and found no problems, and the unit tests cover the most important cases.
Attachment #8750426 - Flags: sec-approval?
Is this too late to get into 47?
status-firefox46: --- → wontfix
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox-esr45: --- → affected
tracking-firefox47: --- → ?
tracking-firefox48: --- → ?
tracking-firefox49: --- → +
tracking-firefox-esr45: --- → ?
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
(In reply to Al Billings [:abillings] from comment #32)
> Is this too late to get into 47?

Yes, as this point in time, I am only open to uplifting sec-high and sec-crit issues. Just based on the rating, this does not meet the Beta47 uplift bar now. Is this easily exploitable? That might make me reconsider. Please let me know.
Flags: needinfo?(rkothari)
(Assignee)

Comment 34

2 years ago
(In reply to Ritu Kothari (:ritu) from comment #33)
> (In reply to Al Billings [:abillings] from comment #32)
> > Is this too late to get into 47?
> 
> Yes, as this point in time, I am only open to uplifting sec-high and
> sec-crit issues. Just based on the rating, this does not meet the Beta47
> uplift bar now. Is this easily exploitable? That might make me reconsider.
> Please let me know.

There is a possible privacy leak if a page initiates a load in its onunload handler.
Also, an attacker may cause performance.getEntries() to report entries that the current page hasn't loaded, by triggering the load in the previous' page onunload handler. It isn't clear how dangerous this is.
From discussion with Al it sounds like it is borderline. So I agree we shouldn't take this at the last minute in beta. But let's land it to nightly and aurora. We could still take it in the next esr that releases along with 48 (45.3.0)
status-firefox47: affected → wontfix
tracking-firefox47: ? → -
tracking-firefox48: ? → +
Flags: needinfo?(lhenry)
Can you request uplift to aurora once this lands on m-c? Thanks.
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 37

2 years ago
Setting checkin needed for attachment 8750426 [details] [diff] [review]
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/88c344a56eac
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(Assignee)

Comment 40

2 years ago
Comment on attachment 8750426 [details] [diff] [review]
Use mLoadInfo to get the performance object

Approval Request Comment
[Feature/regressing bug #]:
Since first implementation of Resource Timing (bug 822480)

[User impact if declined]:
Possible privacy leak. See comment 34.

[Describe test coverage new/current, TreeHerder]:
Existing mochitests and web-platform-tests for resource timing.
Attachment 8748584 [details] [diff] (not checked in yet) tests the correct behaviour of this change.

[Risks and why]: 
Slight risk of regressions (not reporting resource timing entries). Main use cases are covered by tests and have been verified locally.

[String/UUID change made/needed]:
None.
Flags: needinfo?(valentin.gosu)
Attachment #8750426 - Flags: approval-mozilla-aurora?
tracking-firefox-esr45: ? → 48+
Group: dom-core-security → core-security-release
Comment on attachment 8750426 [details] [diff] [review]
Use mLoadInfo to get the performance object

please also land on aurora, a=dveditz.

But don't land the tests yet! I'll set the in-testsuite? flag as a reminder to land those closer to the 48 release.
Attachment #8750426 - Flags: sec-approval?
Attachment #8750426 - Flags: sec-approval+
Attachment #8750426 - Flags: approval-mozilla-aurora?
Attachment #8750426 - Flags: approval-mozilla-aurora+
Flags: in-testsuite?
Whiteboard: maybe sec-high? btpp-active → [land tests in July] maybe sec-high? btpp-active
has problems uplifting to aurora, could you take a look, thanks!

grafting 347531:88c344a56eac "Bug 1254688 - Use mLoadInfo to get the performance object. r=bz, r=tanvi"
merging dom/tests/mochitest/general/resource_timing_main_test.html
merging netwerk/protocol/http/HttpBaseChannel.cpp
warning: conflicts while merging netwerk/protocol/http/HttpBaseChannel.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 43

2 years ago
Created attachment 8759617 [details] [diff] [review]
Use mLoadInfo to get the performance object [aurora]

MozReview-Commit-ID: KMbwR7J8FLm
(Assignee)

Comment 44

2 years ago
I rebased the patch for aurora.
Flags: needinfo?(valentin.gosu)
Hi Catalin, can you verify that this has been fixed on your end? 

You can try a build from here: 
http://ftp.mozilla.org/pub/firefox/candidates/48.0b9-candidates/build1/
Flags: needinfo?(catalind)
Catalin, can you tell me who the "we" for bug credit in advisories is for this issue?
Alias: CVE-2016-5250
Whiteboard: [land tests in July] maybe sec-high? btpp-active → [land tests in July][adv-main48+] maybe sec-high? btpp-active
This is supposed to go on the ESR-45 branch for the Fx48-matching release (see status flags). Why hasn't this had an ESR-approval request (likely needs a separate patch) and landed yet?
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 50

2 years ago
Created attachment 8775389 [details] [diff] [review]
Use mLoadInfo to get the performance object [esr45]

MozReview-Commit-ID: KMbwR7J8FLm
(Assignee)

Comment 51

2 years ago
Comment on attachment 8775389 [details] [diff] [review]
Use mLoadInfo to get the performance object [esr45]

[Approval Request Comment]
User impact if declined: Possible privacy leak. See comment 34.
Fix Landed on Version: 49, uplifted to 48.
Risk to taking this patch (and alternatives if risky): Low risk. No known regressions in the past few months.
String or UUID changes made by this patch: none
Flags: needinfo?(valentin.gosu)
Attachment #8775389 - Flags: approval-mozilla-esr45?
Must be ESR45.4 now. This missed the board for 45.3.
tracking-firefox-esr45: 48+ → 49+
Comment on attachment 8775389 [details] [diff] [review]
Use mLoadInfo to get the performance object [esr45]

Doesn't match the esr criteria, however, as it has a cve, taking it.
Should be in 45.4
Attachment #8775389 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
https://hg.mozilla.org/releases/mozilla-esr45/rev/6711ccb0184e
status-firefox-esr45: affected → fixed
Whiteboard: [land tests in July][adv-main48+] maybe sec-high? btpp-active → [land tests in July][adv-main48+][adv-esr45.4+] maybe sec-high? btpp-active
Group: core-security-release
(Assignee)

Comment 55

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/142c2979af94204afd0963202c4b8f5282dfd38e
Bug 1254688 - Test that loaded in onunload shouldn't show up in the next page performance object r=bz
Backed out last push for failing mochitest dom/tests/mochitest/general/test_resource_timing_unload.html | Only one entry should be found:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8912096e79cde3931d84d8e2e0257518f4b7d001

Push which ran failing test: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b382ec54d164fde891d5f520fd15654d1d521ba9&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130666428&repo=mozilla-inbound

[task 2017-09-13T14:36:57.349167Z] 14:36:57     INFO -  3071 INFO TEST-START | dom/tests/mochitest/general/test_resource_timing_unload.html
[task 2017-09-13T14:36:58.161809Z] 14:36:58     INFO -  TEST-INFO | started process screentopng
[task 2017-09-13T14:36:59.959793Z] 14:36:59     INFO -  TEST-INFO | screentopng: exit 0
[task 2017-09-13T14:36:59.963033Z] 14:36:59    ERROR -  3072 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_resource_timing_unload.html | Only one entry should be found - got +0, expected 1
[task 2017-09-13T14:36:59.964318Z] 14:36:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:312:5
[task 2017-09-13T14:36:59.965724Z] 14:36:59     INFO -      is@dom/tests/mochitest/general/file_resource_timing_unload1.html:12:3
[task 2017-09-13T14:36:59.967174Z] 14:36:59     INFO -      window.onload@dom/tests/mochitest/general/file_resource_timing_unload1.html:17:3
[task 2017-09-13T14:36:59.968469Z] 14:36:59     INFO -      EventHandlerNonNull*@dom/tests/mochitest/general/file_resource_timing_unload1.html:15:1
Flags: needinfo?(valentin.gosu)
(Assignee)

Updated

a year ago
Blocks: 1399535
(Assignee)

Comment 57

a year ago
I filed bug 1399535 to fixup and land the test.
Flags: needinfo?(valentin.gosu)
You need to log in before you can comment on or make changes to this bug.