1308938 - Assertion failure: originAttrsLoadInfo.mUserContextId == originAttrsLoadContext.mUserContextId during loading of about:preferences

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Andrew McCreight [:mccr8]]

The test browser/components/contextualidentity/test/browser/browser_aboutURLs.js loads every about: page. It is currently disabled on Windows and Linux debug for an intermittent negative leak (bug 1271182). I tried reenabling it to see what would happen, and it turns out there's an assertion failure every time it is run (unrelated to the negative leak):

Assertion failure: originAttrsLoadInfo.mUserContextId == originAttrsLoadContext.mUserContextId (The value of mUserContextId in the loadContext and in the loadInfo are not the same!), at /home/worker/workspace/build/src/netwerk/base/nsNetUtil.cpp:2447
#01: nsBaseChannel::AsyncOpen [netwerk/base/nsBaseChannel.cpp:660]
#02: nsBaseChannel::AsyncOpen2 [netwerk/base/nsBaseChannel.cpp:704]
#03: imgLoader::LoadImage [image/imgLoader.cpp:2206]
#04: nsContentUtils::LoadImage [dom/base/nsContentUtils.cpp:3306]
#05: nsImageBoxFrame::UpdateImage [layout/xul/nsImageBoxFrame.cpp:241]
#06: nsImageBoxFrame::AttributeChanged [layout/xul/nsImageBoxFrame.cpp:139]
#07: mozilla::RestyleManager::AttributeChanged [xpcom/glue/nsTArray.h:878]
#08: PresShell::AttributeChanged [layout/base/RestyleManagerHandleInlines.h:150]
#09: nsNodeUtils::AttributeChanged [dom/base/nsNodeUtils.cpp:145]
#10: mozilla::dom::Element::SetAttrAndNotify [dom/base/Element.cpp:2548]

It looks like this happens when the test loads about:preferences.

I don't know if this is a security issue or not, so I'm filing this hidden.

Here's a log with the failure:
https://treeherder.mozilla.org/logviewer.html#?job_id=28866297&repo=try

Comment 1

[Andrew McCreight [:mccr8]]

Dragana, do you know how bad of an assertion this is? Thanks.

Comment 2

[Andrew McCreight [:mccr8]]

This assertion shows up on Linux64, but not OSX. I haven't tried Windows.

Comment 3

[Dragana Damjanovic [:dragana]]

(In reply to Andrew McCreight [:mccr8] from comment #1)
> Dragana, do you know how bad of an assertion this is? Thanks.

Christoph is a better person to ask.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

An easier STR is to open about:preference in a container tab.
And it asserts because the loadInfo->LoadingPrincipal() is SystemPrincical, hence no userContextId is stored on the origin attributes, however I remembered this worked before.

Kamil, can you help to look which bug regresses this?

Thanks

Comment 5

[Andrew McCreight [:mccr8]]

This assertions seems to only happen on Linux. Windows and OSX are both fine.

Comment 6

[Andrew McCreight [:mccr8]]

The test was disabled on Linux in bug 1271182 comment 75, which hit inbound July 27.

Comment 7

[Kamil Jozwiak [:kjozwiak]]

I tried going through mozregression using "--build-type debug" under Ubuntu 16.04. Unfortunately I kept receiving "ERROR: Unable to find build info using the taskcluster route" error so I ended up going through this process manually using the following inbound builds:
* https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-inbound.pushdate.2016.08.05/gecko.v2.mozilla-inbound.pushdate.2016.08.05

Results:
========

20160805094455 - good
20160805094630 - good
20160805100501 - good
20160805103349 - good
20160805115216 - bad
20160805130059 - bad
20160805132614 - bad
20160805134925 - bad

Regression range:
* https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4a431f553e0c8bc5e005f5c1684d4a1da026a47a&tochange=46d487ebf9afe289928ca424071bb58a870c65cd

Possible suspect:
* https://bugzilla.mozilla.org/show_bug.cgi?id=1264231

Yoshi, mind taking a look and seeing if bug#1264231 caused this regression?

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #7)
> Yoshi, mind taking a look and seeing if bug#1264231 caused this regression?
Yeah, that's it. 
Thanks for finding this.

Comment 9

[Kamil Jozwiak [:kjozwiak]]

Attachment: complete stack of crash

I'm running into the following crash when opening about:preferences in the default container while another website is also being loaded in the personal container at the same time. I can consistently reproduce under macOS 10.12 using xCode 8.0 (8A218a).

STR:

* launch the latest version m-c debug
* open a personal container via File -> New Container Tab
* open a regular tab via File -> New Tab
* load gmail.com within the personal container
* while gmail.com is loading, load about:preference in the regular tab (default container)

Yoshi, could this be related to this bug as it involves about:preferences? Or should I spawn this into it's own separate bug?

Assertion failure: docShellAttrs.mUserContextId == attrs.mUserContextId (docshell and necko should have the same userContextId attribute.), at /Users/kjozwiak/projects/m-c/netwerk/protocol/http/HttpBaseChannel.cpp:2948
#01: mozilla::net::HttpBaseChannel::SetupReplacementChannel(nsIURI*, nsIChannel*, bool, unsigned int)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x877c24]
#02: mozilla::net::nsHttpChannel::SetupReplacementChannel(nsIURI*, nsIChannel*, bool, unsigned int)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8ed6f9]
#03: mozilla::net::nsHttpChannel::ContinueProcessRedirectionAfterFallback(nsresult)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8f7050]
#04: mozilla::net::nsHttpChannel::AsyncProcessRedirection(unsigned int)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8df584]
#05: mozilla::net::nsHttpChannel::ContinueProcessResponse1(nsresult)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8e882d]
#06: mozilla::net::nsHttpChannel::ProcessResponse()[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8e83f7]
#07: mozilla::net::nsHttpChannel::OnStartRequest(nsIRequest*, nsISupports*)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8ff174]
#08: non-virtual thunk to mozilla::net::nsHttpChannel::OnStartRequest(nsIRequest*, nsISupports*)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8ff837]
#09: nsInputStreamPump::OnStateStart()[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x35f395]
#10: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x35eebd]
#11: non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x36002c]
#12: nsInputStreamReadyEvent::Run()[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x190329]
#13: nsThread::ProcessNextEvent(bool, bool*)[/Users/kjozwiak/projects/m-c/objdir-ff-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1cdc19]

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #9)
> I'm running into the following crash when opening about:preferences in the
> default container while another website is also being loaded in the personal
> container at the same time. I can consistently reproduce under macOS 10.12
> using xCode 8.0 (8A218a).
> 
Should be a different bug. I'll check this.

Comment 11

[Tanvi Vyas[:tanvi]]

Does this need to be a security sensitive bug?

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch.

waiting for mccr8's response in https://bugzilla.mozilla.org/show_bug.cgi?id=1271182#c128 first before sending r?

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

Comment 13

[Kamil Jozwiak [:kjozwiak]]

(In reply to Yoshi Huang[:allstars.chh] from comment #10)
> Should be a different bug. I'll check this.

Created bug#1310276.

Comment 14

[Andrew McCreight [:mccr8]]

(In reply to Tanvi Vyas [:tanvi] from comment #11)
> Does this need to be a security sensitive bug?

I have no idea if it needs to be security sensitive or not. I just saw the principal stuff and was concerned that it might be sensitive.

Comment 15

[Andrea Marchesini [:baku]]

Comment on attachment 8800987 [details] [diff] [review]
Patch.

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

::: netwerk/base/nsNetUtil.cpp
@@ +2401,5 @@
>        nsCString spec = uri->GetSpecOrDefault();
>        isAboutPage = spec.EqualsLiteral("about:newtab") ||
> +                    spec.EqualsLiteral("about:sync-tabs") ||
> +                    spec.EqualsLiteral("about:preferences") ||
> +                    spec.EqualsLiteral("about:addons");

should we generalize this a bit more? Should we use the scheme?

Comment 16

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v2

use scheme to check.

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8802052 [details] [diff] [review]
Patch. v2

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

Hi, Baku
Thanks for your review, this time I use scheme to check the URI.
Can you review this again?

Thanks

Comment 18

[Andrea Marchesini [:baku]]

Comment on attachment 8802052 [details] [diff] [review]
Patch. v2

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

::: netwerk/base/nsNetUtil.cpp
@@ +2397,5 @@
>    if (node) {
>      nsIDocument* doc = node->OwnerDoc();
>      if (doc) {
>        nsIURI* uri = doc->GetDocumentURI();
> +      nsAutoCString scheme;

uri->SchemeIs("about", &isAboutPage);

Comment 19

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v3

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59548efdaa50

Comment 21

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I'll request uplift to 51 as this is regressed from bug 1264231.

Comment 22

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

The flag is "status-firefox51: 	affected", so it should be uplifted soon.

Comment 23

[Gerry Chang [:gchang]]

Hi :allstars.chh,
Please create a uplift request for 51 aurora.

Comment 24

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8802410 [details] [diff] [review]
Patch. v3

Approval Request Comment

[Feature/regressing bug #]:
bug 1264231

[User impact if declined]:
Open about:preferences in a container tab will crash in debug build.

[Describe test coverage new/current, TreeHerder]:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b38b3df8e2ac

[Risks and why]:
Low, the assertion are used to check the loading in content side, for about: pages it's done in chrome side, so we should skip the check if it's about: pages.

[String/UUID change made/needed]:
No

Comment 25

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8802410 [details] [diff] [review]
Patch. v3

Forgot to mention that,
Please uplift this after bug 1271182 has been uplifted to firefox51, otherwise my patch will have a conflict here.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1271182#c131
https://bugzilla.mozilla.org/show_bug.cgi?id=1271182#c132

Thanks

Comment 26

[Gerry Chang [:gchang]]

Comment on attachment 8802410 [details] [diff] [review]
Patch. v3

Fix a sec-moderate issue. Take it in 51 aurora.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/10be9d40fa865be7c3c203b9cd042722ab3069ca

Comment 28

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following test cases:

* opened about:preferences in several containers (all at the same time as well)
* opened about:preferences within the default container
* opened about:preferences within PB
* opened all the about: sites listed under about:about in a container

Builds being used:

* fx52.0a1 (--enable-debug), buildId: 20161111172704, changeset: fc104971a4db
* fx51.0a2 (--enable-debug), buildId: 20161111183120, changeset: 3d380055aaed