Closed Bug 1108547 Opened 10 years ago Closed 10 years ago

Private browsing mode context is broken by <a> or <form> with target attribute

Categories

(Firefox :: Private Browsing, defect)

37 Branch
x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 38

People

(Reporter: sdna.muneaki.nishimura, Assigned: ehsan.akhgari)

References

Details

(Keywords: privacy)

Attachments

(3 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.28 Safari/537.36 Steps to reproduce: 1) Launch following page from normal browsing mode in order to set cookie http://csrf.jp/cookie/set.php?domain=csrf.jp&name=NON-PRIVATE&value=MODE-COOKIE 2) Open private browsing mode and launch following page http://csrf.jp/target.html 3) Click "Non-Private Mode Cookie is Used" link or button Actual results: At procedure 3), non-browsing mode cookie is shown on an alert dialog. Expected results: At procedure 3), private mode cookie has to be shown. Note that the text/button of "Non-Private Mode Cookie is Used" has target attribute.
I confirm this behavior in nightly: even though the new page is a tab in a window labelled with the private browsing icon, it is NOT a private tab. Of course putting it in a correctly labelled window isn't the solution because this lets private pages collaborate with their public counter-parts. Ehsan: can you help route this bug to whomever owns private browsing these days?
Component: Untriaged → Private Browsing
Flags: needinfo?(ehsan.akhgari)
Keywords: privacy
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: sec-bounty?
Here is the supplement on the bug: I'm not sure whether the cause is same but hyperlink with 'target' attribute can access other window/frame beyond the private browsing mode like below. 1) Open http://alice.csrf.jp/iframe/public.html on normal mode. 2) Open http://alice.csrf.jp/iframe/private.html on private mode. 3) Click a link 'Change Public Frame' on 2). 4) Child page in 1) is changed beyond the mode. The opposite is equally possible, i.e., private window can be controlled from normal mode.
(In reply to Muneaki Nishimura from comment #2) > Here is the supplement on the bug: > I'm not sure whether the cause is same but hyperlink with 'target' attribute > can access other window/frame beyond the private browsing mode like below. > > 1) Open http://alice.csrf.jp/iframe/public.html on normal mode. > 2) Open http://alice.csrf.jp/iframe/private.html on private mode. > 3) Click a link 'Change Public Frame' on 2). > 4) Child page in 1) is changed beyond the mode. > > The opposite is equally possible, i.e., private window can be controlled > from normal mode. This is covered by bug 1100154, I think. Josh, I know you made that a mentored bug, but this seems like it's more serious and might be caused by the same issue... in which case, do you have time to look at this?
Flags: needinfo?(josh)
Assignee: nobody → ehsan.akhgari
Flags: needinfo?(ehsan.akhgari)
I don't think this is a security bug. Is it OK to open it up?
Flags: needinfo?(dveditz)
Group: core-security
Flags: needinfo?(dveditz)
(In reply to :Gijs Kruitbosch from comment #3) > (In reply to Muneaki Nishimura from comment #2) > > Here is the supplement on the bug: > > I'm not sure whether the cause is same but hyperlink with 'target' attribute > > can access other window/frame beyond the private browsing mode like below. > > > > 1) Open http://alice.csrf.jp/iframe/public.html on normal mode. > > 2) Open http://alice.csrf.jp/iframe/private.html on private mode. > > 3) Click a link 'Change Public Frame' on 2). > > 4) Child page in 1) is changed beyond the mode. > > > > The opposite is equally possible, i.e., private window can be controlled > > from normal mode. > > This is covered by bug 1100154, I think. I'm pretty sure these are both the same underlying issue, but will know for sure when I finish my fix for this bug.
The problem here seems to be how the docshell for the new tab is initialized: * thread #1: tid = 0x3d20d0, 0x000000010559fd67 XUL`nsDocShell::SetDocLoaderParent(this=0x000000011e1af800, aParent=0x00000001003270b0) + 1719 at nsDocShell.cpp:3399, queue = 'com.apple.main-thread', stop reason = step over frame #0: 0x000000010559fd67 XUL`nsDocShell::SetDocLoaderParent(this=0x000000011e1af800, aParent=0x00000001003270b0) + 1719 at nsDocShell.cpp:3399 frame #1: 0x0000000102817a04 XUL`nsDocLoader::AddChildLoader(this=0x00000001003270b0, aChild=0x000000011e1af800) + 148 at nsDocLoader.cpp:644 * frame #2: 0x0000000102817947 XUL`nsDocLoader::AddDocLoaderAsChildOfRoot(aDocLoader=0x000000011e1af800) + 391 at nsDocLoader.cpp:248 frame #3: 0x000000010558c208 XUL`nsDocShell::Init(this=0x000000011e1af800) + 888 at nsDocShell.cpp:954 frame #4: 0x00000001055f219d XUL`nsDocShellConstructor(aOuter=0x0000000000000000, aIID=0x0000000107749138, aResult=0x00007fff5fbf2440) + 205 at nsDocShellModule.cpp:68 frame #5: 0x0000000101736cde XUL`mozilla::GenericFactory::CreateInstance(this=0x000000011a803b00, aOuter=0x0000000000000000, aIID=0x0000000107749138, aResult=0x00007fff5fbf2440) + 46 at GenericFactory.cpp:17 frame #6: 0x00000001016d2362 XUL`nsComponentManagerImpl::CreateInstanceByContractID(this=0x00000001003264e0, aContractID=0x00000001084bbe67, aDelegate=0x0000000000000000, aIID=0x0000000107749138, aResult=0x00007fff5fbf2440) + 338 at nsComponentManager.cpp:1199 frame #7: 0x000000010173b42b XUL`CallCreateInstance(aContractID=0x00000001084bbe67, aDelegate=0x0000000000000000, aIID=0x0000000107749138, aResult=0x00007fff5fbf2440) + 139 at nsComponentManagerUtils.cpp:149 frame #8: 0x000000010173b634 XUL`nsCreateInstanceByContractID::operator(this=0x00007fff5fbf2938, aIID=0x0000000107749138, aInstancePtr=0x00007fff5fbf2440)(nsID const&, void**) const + 52 at nsComponentManagerUtils.cpp:197 frame #9: 0x0000000102822e01 XUL`nsCOMPtr<nsIDocShell>::assign_from_helper(this=0x000000011a26b8a8, helper=0x00007fff5fbf2938, aIID=0x0000000107749138) + 65 at nsCOMPtr.h:1109 frame #10: 0x0000000103019cc6 XUL`nsCOMPtr<nsIDocShell>::operator=(this=0x000000011a26b8a8, aRhs=0x00007fff5fbf2938) + 54 at nsCOMPtr.h:622 frame #11: 0x0000000102fd652a XUL`nsFrameLoader::MaybeCreateDocShell(this=0x000000011a26b880) + 650 at nsFrameLoader.cpp:1612 frame #12: 0x0000000102fd54dd XUL`nsFrameLoader::CheckURILoad(this=0x000000011a26b880, aURI=0x000000011bd1e9d0) + 173 at nsFrameLoader.cpp:490 frame #13: 0x0000000102fd5219 XUL`nsFrameLoader::LoadURI(this=0x000000011a26b880, aURI=0x000000011bd1e9d0) + 233 at nsFrameLoader.cpp:287 frame #14: 0x0000000102fd4fff XUL`nsFrameLoader::LoadFrame(this=0x000000011a26b880) + 1231 at nsFrameLoader.cpp:253 frame #15: 0x000000010499f17d XUL`nsXULElement::LoadSrc(this=0x000000011bd1dbc0) + 477 at nsXULElement.cpp:1609 frame #16: 0x000000010499ee03 XUL`nsXULElement::BindToTree(this=0x000000011bd1dbc0, aDocument=0x000000011c38e000, aParent=0x000000011bd1e790, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 675 at nsXULElement.cpp:882 frame #17: 0x0000000102e2da70 XUL`mozilla::dom::Element::BindToTree(this=0x000000011bd1e790, aDocument=0x000000011c38e000, aParent=0x000000011bd1e820, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 3104 at Element.cpp:1539 frame #18: 0x000000010499ec3b XUL`nsXULElement::BindToTree(this=0x000000011bd1e790, aDocument=0x000000011c38e000, aParent=0x000000011bd1e820, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 219 at nsXULElement.cpp:840 frame #19: 0x0000000102e2da70 XUL`mozilla::dom::Element::BindToTree(this=0x000000011bd1e820, aDocument=0x000000011c38e000, aParent=0x000000011bd1e8b0, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 3104 at Element.cpp:1539 frame #20: 0x000000010499ec3b XUL`nsXULElement::BindToTree(this=0x000000011bd1e820, aDocument=0x000000011c38e000, aParent=0x000000011bd1e8b0, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 219 at nsXULElement.cpp:840 frame #21: 0x0000000102e2da70 XUL`mozilla::dom::Element::BindToTree(this=0x000000011bd1e8b0, aDocument=0x000000011c38e000, aParent=0x000000011bd1e940, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 3104 at Element.cpp:1539 frame #22: 0x000000010499ec3b XUL`nsXULElement::BindToTree(this=0x000000011bd1e8b0, aDocument=0x000000011c38e000, aParent=0x000000011bd1e940, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 219 at nsXULElement.cpp:840 frame #23: 0x0000000102e2da70 XUL`mozilla::dom::Element::BindToTree(this=0x000000011bd1e940, aDocument=0x000000011c38e000, aParent=0x000000011c1f8040, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 3104 at Element.cpp:1539 frame #24: 0x000000010499ec3b XUL`nsXULElement::BindToTree(this=0x000000011bd1e940, aDocument=0x000000011c38e000, aParent=0x000000011c1f8040, aBindingParent=0x000000011e2fc6b0, aCompileEventHandlers=true) + 219 at nsXULElement.cpp:840 frame #25: 0x0000000103000870 XUL`nsINode::doInsertChildAt(this=0x000000011c1f8040, aKid=0x000000011bd1e940, aIndex=1, aNotify=true, aChildArray=0x000000011c1f80b0) + 1232 at nsINode.cpp:1525 frame #26: 0x0000000102e4c209 XUL`mozilla::dom::FragmentOrElement::InsertChildAt(this=0x000000011c1f8040, aKid=0x000000011bd1e940, aIndex=1, aNotify=true) + 137 at FragmentOrElement.cpp:1140 frame #27: 0x0000000103002b87 XUL`nsINode::ReplaceOrInsertBefore(this=0x000000011c1f8040, aReplace=false, aNewChild=0x000000011bd1e940, aRefChild=0x0000000000000000, aError=0x00007fff5fbf3dc8) + 5383 at nsINode.cpp:2209 frame #28: 0x0000000102e620b4 XUL`nsINode::InsertBefore(this=0x000000011c1f8040, aNode=0x000000011bd1e940, aChild=0x0000000000000000, aError=0x00007fff5fbf3dc8) + 52 at nsINode.h:1600 frame #29: 0x0000000102e620f2 XUL`nsINode::AppendChild(this=0x000000011c1f8040, aNode=0x000000011bd1e940, aError=0x00007fff5fbf3dc8) + 50 at nsINode.h:1604 frame #30: 0x000000010386ffdb XUL`mozilla::dom::NodeBinding::appendChild(cx=0x0000000125dc9350, obj=Handle<JSObject *> at 0x00007fff5fbf3e20, self=0x000000011c1f8040, args=0x00007fff5fbf3e90) + 379 at NodeBinding.cpp:598 frame #31: 0x0000000103d70134 XUL`mozilla::dom::GenericBindingMethod(cx=0x0000000125dc9350, argc=1, vp=0x0000000116ef5460) + 660 at BindingUtils.cpp:2434 frame #32: 0x000000010727e794 XUL`js::CallJSNative(cx=0x0000000125dc9350, native=0x0000000103d6fea0, args=0x00007fff5fbf4520)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 164 at jscntxtinlines.h:231 frame #33: 0x000000010720028d XUL`js::Invoke(cx=0x0000000125dc9350, args=CallArgs at 0x00007fff5fbf4520, construct=NO_CONSTRUCT) + 1261 at Interpreter.cpp:484 frame #34: 0x0000000107221013 XUL`Interpret(cx=0x0000000125dc9350, state=0x00007fff5fbf7140) + 51379 at Interpreter.cpp:2541 frame #35: 0x000000010721470a XUL`js::RunScript(cx=0x0000000125dc9350, state=0x00007fff5fbf7140) + 666 at Interpreter.cpp:434 frame #36: 0x00000001072003d1 XUL`js::Invoke(cx=0x0000000125dc9350, args=CallArgs at 0x00007fff5fbf7630, construct=NO_CONSTRUCT) + 1585 at Interpreter.cpp:503 frame #37: 0x00000001071eae24 XUL`js::Invoke(cx=0x0000000125dc9350, thisv=0x00007fff5fbf7840, fval=0x00007fff5fbf80c0, argc=4, argv=0x00007fff5fbf82e8, rval=JS::MutableHandleValue at 0x00007fff5fbf7730) + 900 at Interpreter.cpp:540 frame #38: 0x0000000106fb6921 XUL`JS_CallFunctionValue(cx=0x0000000125dc9350, obj=JS::HandleObject at 0x00007fff5fbf7888, fval=JS::HandleValue at 0x00007fff5fbf7880, args=0x00007fff5fbf7dc0, rval=JS::MutableHandleValue at 0x00007fff5fbf7878) + 337 at jsapi.cpp:4931 frame #39: 0x0000000102479b98 XUL`nsXPCWrappedJSClass::CallMethod(this=0x0000000126129a10, wrapper=0x0000000126230c80, methodIndex=3, info_=0x000000011690d688, nativeParams=0x00007fff5fbf8550) + 6392 at XPCWrappedJSClass.cpp:1187 frame #40: 0x0000000102473a1b XUL`nsXPCWrappedJS::CallMethod(this=0x0000000126230c80, methodIndex=3, info=0x000000011690d688, params=0x00007fff5fbf8550) + 203 at XPCWrappedJS.cpp:532 frame #41: 0x0000000101716f16 XUL`PrepareAndDispatch(self=0x000000011e3af800, methodIndex=3, args=0x00007fff5fbf8670, gpregs=0x00007fff5fbf85f0, fpregs=0x00007fff5fbf8620) + 1654 at xptcstubs_x86_64_darwin.cpp:122 frame #42: 0x000000010171592b XUL`SharedStub + 91 frame #43: 0x000000010566d6a1 XUL`nsContentTreeOwner::ProvideWindow(this=0x000000011c1f8430, aParent=0x000000011e579420, aChromeFlags=66766, aCalledFromJS=false, aPositionSpecified=false, aSizeSpecified=false, aURI=0x000000011c1ad080, aName=0x00007fff5fbf91b0, aFeatures=0x00007fff5fbf9330, aWindowIsNew=0x00007fff5fbf9260, aReturn=0x00007fff5fbf9068) + 2001 at nsContentTreeOwner.cpp:922 frame #44: 0x000000010566d820 XUL`non-virtual thunk to nsContentTreeOwner::ProvideWindow(this=0x000000011c1f8450, aParent=0x000000011e579420, aChromeFlags=66766, aCalledFromJS=false, aPositionSpecified=false, aSizeSpecified=false, aURI=0x000000011c1ad080, aName=0x00007fff5fbf91b0, aFeatures=0x00007fff5fbf9330, aWindowIsNew=0x00007fff5fbf9260, aReturn=0x00007fff5fbf9068) + 208 at nsContentTreeOwner.cpp:925 frame #45: 0x00000001055f6dc8 XUL`nsWindowWatcher::OpenWindowInternal(this=0x00000001183a0e00, aParent=0x000000011e579420, aUrl=0x00000001260abbe8, aName=0x00007fff5fbf98a0, aFeatures=0x00007fff5fbf9900, aCalledFromJS=false, aDialog=false, aNavigate=false, aOpeningTab=0x0000000000000000, argv=0x0000000000000000, _retval=0x00007fff5fbf9770) + 3992 at nsWindowWatcher.cpp:627 frame #46: 0x00000001055f907c XUL`nsWindowWatcher::OpenWindow2(this=0x00000001183a0e00, aParent=0x000000011e579420, aUrl=0x00000001260abbe8, aName=0x00007fff5fbf98a0, aFeatures=0x00007fff5fbf9900, aCalledFromScript=false, aDialog=false, aNavigate=false, aOpeningTab=0x0000000000000000, aArguments=0x0000000000000000, _retval=0x00007fff5fbf9770) + 476 at nsWindowWatcher.cpp:418 frame #47: 0x00000001055f917e XUL`non-virtual thunk to nsWindowWatcher::OpenWindow2(this=0x00000001183a0e08, aParent=0x000000011e579420, aUrl=0x00000001260abbe8, aName=0x00007fff5fbf98a0, aFeatures=0x00007fff5fbf9900, aCalledFromScript=false, aDialog=false, aNavigate=false, aOpeningTab=0x0000000000000000, aArguments=0x0000000000000000, _retval=0x00007fff5fbf9770) + 206 at nsWindowWatcher.cpp:421 frame #48: 0x0000000102d7e765 XUL`nsGlobalWindow::OpenInternal(this=0x000000011e579400, aUrl=0x00007fff5fbfa2f0, aName=0x00007fff5fbfa438, aOptions=0x00007fff5fbfa390, aDialog=false, aContentModal=false, aCalledNoScript=true, aDoJSFixups=false, aNavigate=false, argv=0x0000000000000000, aExtraArgument=0x0000000000000000, aCalleePrincipal=0x000000011c0faf10, aJSCallerContext=0x0000000000000000, aReturn=0x00007fff5fbfa430) + 3013 at nsGlobalWindow.cpp:11839 frame #49: 0x0000000102d7f174 XUL`nsGlobalWindow::OpenNoNavigate(this=0x000000011e579400, aUrl=0x00007fff5fbfa2f0, aName=0x00007fff5fbfa438, aOptions=0x00007fff5fbfa390, _retval=0x00007fff5fbfa430) + 308 at nsGlobalWindow.cpp:7677 frame #50: 0x0000000102d7f1c7 XUL`non-virtual thunk to nsGlobalWindow::OpenNoNavigate(this=0x000000011e579420, aUrl=0x00007fff5fbfa2f0, aName=0x00007fff5fbfa438, aOptions=0x00007fff5fbfa390, _retval=0x00007fff5fbfa430) + 71 at nsGlobalWindow.cpp:7688 frame #51: 0x00000001055940c0 XUL`nsDocShell::InternalLoad(this=0x000000011df84000, aURI=0x0000000125d78900, aReferrer=0x00000001333c9400, aReferrerPolicy=0, aOwner=0x000000011c0faf10, aFlags=0, aWindowTarget=0x00007fff5fbfaae8, aTypeHint=0x000000010761005c, aFileName=0x000000010a1b1818, aPostData=0x000000011e374900, aHeadersData=0x0000000000000000, aLoadType=2097153, aSHEntry=0x0000000000000000, aFirstParty=true, aSrcdoc=0x000000010a1b1818, aSourceDocShell=0x000000011df841a8, aBaseURI=0x0000000000000000, aDocShell=0x00007fff5fbfb0b8, aRequest=0x000000012c5eb978) + 4784 at nsDocShell.cpp:9564 frame #52: 0x00000001055c9fce XUL`nsDocShell::OnLinkClickSync(this=0x000000011df84000, aContent=0x000000012c5eb800, aURI=0x0000000125d78680, aTargetSpec=0x000000011e3c05c8, aFileName=0x000000010a1b1818, aPostDataStream=0x000000011e374900, aHeadersDataStream=0x0000000000000000, aDocShell=0x00007fff5fbfb0b8, aRequest=0x000000012c5eb978) + 2654 at nsDocShell.cpp:13255 frame #53: 0x00000001055ca29b XUL`non-virtual thunk to nsDocShell::OnLinkClickSync(this=0x000000011df84210, aContent=0x000000012c5eb800, aURI=0x0000000125d78680, aTargetSpec=0x000000011e3c05c8, aFileName=0x000000010a1b1818, aPostDataStream=0x000000011e374900, aHeadersDataStream=0x0000000000000000, aDocShell=0x00007fff5fbfb0b8, aRequest=0x000000012c5eb978) + 139 at nsDocShell.cpp:13278 frame #54: 0x000000010401c598 XUL`mozilla::dom::HTMLFormElement::SubmitSubmission(this=0x000000012c5eb800, aFormSubmission=0x0000000125ed9f60) + 1720 at HTMLFormElement.cpp:828 frame #55: 0x000000010401bbe8 XUL`mozilla::dom::HTMLFormElement::DoSubmit(this=0x000000012c5eb800, aEvent=0x00007fff5fbfb9b8) + 584 at HTMLFormElement.cpp:686 frame #56: 0x000000010401a94f XUL`mozilla::dom::HTMLFormElement::DoSubmitOrReset(this=0x000000012c5eb800, aEvent=0x00007fff5fbfb9b8, aMessage=1200) + 191 at HTMLFormElement.cpp:606 frame #57: 0x000000010401b88e XUL`mozilla::dom::HTMLFormElement::PostHandleEvent(this=0x000000012c5eb800, aVisitor=0x00007fff5fbfb548) + 238 at HTMLFormElement.cpp:559 frame #58: 0x0000000103f30d21 XUL`mozilla::EventTargetChainItem::PostHandleEvent(this=0x000000012ec5e008, aVisitor=0x00007fff5fbfb548) + 97 at EventDispatcher.cpp:255 frame #59: 0x0000000103f30fea XUL`mozilla::EventTargetChainItem::HandleEventTargetChain(aChain=0x00007fff5fbfb618, aVisitor=0x00007fff5fbfb548, aCallback=0x0000000000000000, aCd=0x00007fff5fbfb620) + 698 at EventDispatcher.cpp:302 frame #60: 0x0000000103f312a4 XUL`mozilla::EventTargetChainItem::HandleEventTargetChain(aChain=0x00007fff5fbfb618, aVisitor=0x00007fff5fbfb548, aCallback=0x0000000000000000, aCd=0x00007fff5fbfb620) + 1396 at EventDispatcher.cpp:348 frame #61: 0x0000000103f32847 XUL`mozilla::EventDispatcher::Dispatch(aTarget=0x000000012c5eb800, aPresContext=0x0000000126553000, aEvent=0x00007fff5fbfb9b8, aDOMEvent=0x0000000000000000, aEventStatus=0x00007fff5fbfb9b4, aCallback=0x0000000000000000, aTargets=0x0000000000000000) + 4935 at EventDispatcher.cpp:633 frame #62: 0x0000000104f82602 XUL`PresShell::HandleDOMEventWithTarget(this=0x000000012c50f000, aTargetContent=0x000000012c5eb800, aEvent=0x00007fff5fbfb9b8, aStatus=0x00007fff5fbfb9b4) + 162 at nsPresShell.cpp:8421 frame #63: 0x0000000104045eb6 XUL`mozilla::dom::HTMLInputElement::PostHandleEvent(this=0x000000012d344e60, aVisitor=0x00007fff5fbfc278) + 8262 at HTMLInputElement.cpp:4275 frame #64: 0x0000000103f30d21 XUL`mozilla::EventTargetChainItem::PostHandleEvent(this=0x000000011a8f3008, aVisitor=0x00007fff5fbfc278) + 97 at EventDispatcher.cpp:255 frame #65: 0x0000000103f30fea XUL`mozilla::EventTargetChainItem::HandleEventTargetChain(aChain=0x00007fff5fbfc348, aVisitor=0x00007fff5fbfc278, aCallback=0x00007fff5fbfc608, aCd=0x00007fff5fbfc350) + 698 at EventDispatcher.cpp:302 frame #66: 0x0000000103f312a4 XUL`mozilla::EventTargetChainItem::HandleEventTargetChain(aChain=0x00007fff5fbfc348, aVisitor=0x00007fff5fbfc278, aCallback=0x00007fff5fbfc608, aCd=0x00007fff5fbfc350) + 1396 at EventDispatcher.cpp:348 frame #67: 0x0000000103f32847 XUL`mozilla::EventDispatcher::Dispatch(aTarget=0x000000012d344e60, aPresContext=0x0000000126553000, aEvent=0x00007fff5fbfca08, aDOMEvent=0x0000000000000000, aEventStatus=0x00007fff5fbfdd18, aCallback=0x00007fff5fbfc608, aTargets=0x0000000000000000) + 4935 at EventDispatcher.cpp:633 frame #68: 0x0000000104f815d7 XUL`PresShell::HandleEventInternal(this=0x000000012c50f000, aEvent=0x00007fff5fbfca08, aStatus=0x00007fff5fbfdd18) + 4871 at nsPresShell.cpp:8251 frame #69: 0x0000000104f81a2c XUL`PresShell::HandleEventWithTarget(this=0x000000012c50f000, aEvent=0x00007fff5fbfca08, aFrame=0x00000001335ca338, aContent=0x000000012d344e60, aStatus=0x00007fff5fbfdd18) + 508 at nsPresShell.cpp:7984 frame #70: 0x0000000103f08d87 XUL`mozilla::EventStateManager::CheckForAndDispatchClick(this=0x000000012e0ffda0, aPresContext=0x0000000126553000, aEvent=0x00007fff5fbfe070, aStatus=0x00007fff5fbfdd18) + 935 at EventStateManager.cpp:4414 frame #71: 0x0000000103f07349 XUL`mozilla::EventStateManager::PostHandleEvent(this=0x000000012e0ffda0, aPresContext=0x0000000126553000, aEvent=0x00007fff5fbfe070, aTargetFrame=0x00000001335ca338, aStatus=0x00007fff5fbfdd18) + 4985 at EventStateManager.cpp:2924 frame #72: 0x0000000104f816bb XUL`PresShell::HandleEventInternal(this=0x000000012c50f000, aEvent=0x00007fff5fbfe070, aStatus=0x00007fff5fbfdd18) + 5099 at nsPresShell.cpp:8263 frame #73: 0x0000000104f802a0 XUL`PresShell::HandlePositionedEvent(this=0x000000012c50f000, aTargetFrame=0x00000001335ca338, aEvent=0x00007fff5fbfe070, aEventStatus=0x00007fff5fbfdd18) + 528 at nsPresShell.cpp:7958 frame #74: 0x0000000104f7f06e XUL`PresShell::HandleEvent(this=0x000000011c137000, aFrame=0x00000001003ad4e8, aEvent=0x00007fff5fbfe070, aDontRetargetEvents=false, aEventStatus=0x00007fff5fbfdd18) + 8414 at nsPresShell.cpp:7755 frame #75: 0x0000000104a31e9a XUL`nsViewManager::DispatchEvent(this=0x0000000126513480, aEvent=0x00007fff5fbfe070, aView=0x000000011e3b3fd0, aStatus=0x00007fff5fbfdd18) + 650 at nsViewManager.cpp:775 frame #76: 0x0000000104a2ec1f XUL`nsView::HandleEvent(this=0x000000011e3b3fd0, aEvent=0x00007fff5fbfe070, aUseAttachedEvents=false) + 319 at nsView.cpp:1097 frame #77: 0x0000000104a83b05 XUL`nsChildView::DispatchEvent(this=0x000000011c133c00, event=0x00007fff5fbfe070, aStatus=0x00007fff5fbfdeac) + 1221 at nsChildView.mm:1764 frame #78: 0x0000000104a83bb6 XUL`nsChildView::DispatchWindowEvent(this=0x000000011c133c00, event=0x00007fff5fbfe070) + 54 at nsChildView.mm:1772 frame #79: 0x0000000104a9437f XUL`-[ChildView mouseUp:](self=0x000000011d3e7ae0, _cmd=0x00007fff969cd122, theEvent=0x000000011e3b67a0) + 463 at nsChildView.mm:4972 frame #80: 0x00007fff9677902b AppKit`-[NSWindow _reallySendEvent:] + 759 frame #81: 0x00007fff9620650c AppKit`-[NSWindow sendEvent:] + 368 frame #82: 0x0000000104adb8cd XUL`-[ToolbarWindow sendEvent:](self=0x000000011c1da540, _cmd=0x00007fff969aa6a9, anEvent=0x000000011e3b67a0) + 365 at nsCocoaWindow.mm:3371 frame #83: 0x00007fff961b8096 AppKit`-[NSApplication sendEvent:] + 2238 frame #84: 0x0000000104ac72a2 XUL`-[GeckoNSApplication sendEvent:](self=0x00000001169cc040, _cmd=0x00007fff969aa6a9, anEvent=0x000000011e3b67a0) + 146 at nsAppShell.mm:107 frame #85: 0x00007fff96044e98 AppKit`-[NSApplication run] + 711 frame #86: 0x0000000104ac9197 XUL`nsAppShell::Run(this=0x0000000100309ce0) + 167 at nsAppShell.mm:651 frame #87: 0x00000001059a098c XUL`nsAppStartup::Run(this=0x0000000116e1c740) + 156 at nsAppStartup.cpp:281 frame #88: 0x0000000105a4eff0 XUL`XREMain::XRE_mainRun(this=0x00007fff5fbfeec8) + 6208 at nsAppRunner.cpp:4150 frame #89: 0x0000000105a4f89e XUL`XREMain::XRE_main(this=0x00007fff5fbfeec8, argc=5, argv=0x00007fff5fbff7f8, aAppData=0x00007fff5fbff178) + 798 at nsAppRunner.cpp:4226 frame #90: 0x0000000105a4fd62 XUL`XRE_main(argc=5, argv=0x00007fff5fbff7f8, aAppData=0x00007fff5fbff178, aFlags=0) + 98 at nsAppRunner.cpp:4446 frame #91: 0x0000000100002d3e firefox`do_main(argc=5, argv=0x00007fff5fbff7f8, xreDirectory=0x000000010030e040) + 1950 at nsBrowserApp.cpp:292 frame #92: 0x00000001000020a3 firefox`main(argc=5, argv=0x00007fff5fbff7f8) + 323 at nsBrowserApp.cpp:661 frame #93: 0x0000000100001b04 firefox`start + 52 Basically, the aParent argument to SetDocLoaderParent is not a docshell, so we cannot inherit the PB flag from it. Later on, we get another call to SetDocLoaderParent which provides an nsDocLoader that is a docshell but it's too late by that time.
Attached patch WIP for bug 1108547 (obsolete) (deleted) — Splinter Review
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #5) > I don't think this is a security bug. Is it OK to open it up? Minusing this for bounty and removing rating based on this.
Flags: sec-bounty? → sec-bounty-
Keywords: sec-moderate
Josh: ping?
Ehsan's working on this, and we discussed his question that wasn't left here as a comment.
Flags: needinfo?(josh)
I talked to Josh about this in person. The best idea we came up with was to extend nsDocShell::SetDocLoaderParent with a second argument (nsDocLoader* aSettingsParent or some such) will has to be non-null when calling this through nsDocShell::Init. nsDocShell::Init itself of course has no idea what the correct settings parent should be, but nsFrameLoader::MaybeCreateDocShell can supply that information through mOwnerContent.
(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #12) > I talked to Josh about this in person. The best idea we came up with was to > extend nsDocShell::SetDocLoaderParent with a second argument (nsDocLoader* > aSettingsParent or some such) will has to be non-null when calling this > through nsDocShell::Init. nsDocShell::Init itself of course has no idea > what the correct settings parent should be, but > nsFrameLoader::MaybeCreateDocShell can supply that information through > mOwnerContent. That was all a huge red herring. :( The issue is that nsHTMLDocument::GetCookie calls nsCookieService::GetCookieString with a null channel, and this check <https://dxr.mozilla.org/mozilla-central/source/netwerk/cookie/nsCookieService.cpp#1620> will happily think that we are in a non-private window. Basically, without a channel, the cookie service doesn't know that we're requesting a private cookie. Whereas without target="_blank", we end up running the javascript: URI code within the context of the existing tab, where nsDocument::mChannel points to an existing channel which has the correct privacy bit set. The only way that I can think of to fix this is to extend nsICookieService::GetCookieString with a privacy flag and pass the right thing from the document.
Except that juggling a boolean flag with the privacy status of the channel really sucks. I'm thinking perhaps only honoring aIsPrivate when the channel is null? Josh, any better ideas?
Flags: needinfo?(josh)
What about creating a dummy channel (if mChannel is null) and calling channel.setPrivate on it? I mean, it's icky, but...
Flags: needinfo?(josh)
(In reply to Josh Matthews [:jdm] from comment #15) > What about creating a dummy channel (if mChannel is null) and calling > channel.setPrivate on it? I mean, it's icky, but... That idea makes me sick to my stomach. :( But I'll try it, I guess it's better than messing with the cookie service.
This ensures that the cookie service can know which cookie database to query from. This is a gross hack, please see the discussion on the bug as to why we did this.
Attached patch Part 3: Automated tests (obsolete) (deleted) — Splinter Review
Attachment #8534055 - Attachment is obsolete: true
Comment on attachment 8552835 [details] [diff] [review] Part 3: Automated tests Oops, a couple of typos caused this to leak two windows until shutdown.
Attachment #8552835 - Attachment is obsolete: true
Attached patch Part 3: Automated tests (deleted) — Splinter Review
Attachment #8552838 - Flags: review?(josh)
Attachment #8552834 - Flags: review?(josh)
Attachment #8552833 - Flags: review?(josh)
Attachment #8552833 - Flags: review?(josh) → review+
Attachment #8552834 - Flags: review?(josh) → review+
Comment on attachment 8552838 [details] [diff] [review] Part 3: Automated tests Review of attachment 8552838 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/html/test/browser_bug1108547.js @@ +62,5 @@ > + privateWin.gBrowser.tabs[privateWin.gBrowser.tabs.length - 1].linkedBrowser.addEventListener("load", onNewTabLoaded, true); > + } > + > + function onNewTabLoaded() { > + privateWin.gBrowser.selectedBrowser.removeEventListener("load", onNewTabLoaded, true); Is selectedBrowser the right one here? Should we use it for adding the listener too? @@ +104,5 @@ > + gBrowser.tabs[gBrowser.tabs.length - 1].linkedBrowser.addEventListener("load", onNewTabLoaded2, true); > + } > + > + function onNewTabLoaded2() { > + gBrowser.selectedBrowser.removeEventListener("load", onNewTabLoaded2, true); Same questions as previously.
Attachment #8552838 - Flags: review?(josh) → review+
Comment on attachment 8552838 [details] [diff] [review] Part 3: Automated tests Review of attachment 8552838 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/html/test/browser_bug1108547.js @@ +62,5 @@ > + privateWin.gBrowser.tabs[privateWin.gBrowser.tabs.length - 1].linkedBrowser.addEventListener("load", onNewTabLoaded, true); > + } > + > + function onNewTabLoaded() { > + privateWin.gBrowser.selectedBrowser.removeEventListener("load", onNewTabLoaded, true); Hmm, yeah perhaps that is unreliable... I'll fix it to use privateWin.gBrowser.tabs[privateWin.gBrowser.tabs.length - 1].linkedBrowser.
The failure is Linux specific.
>TEST-UNEXPECTED-PASS | /html/dom/documents/resource-metadata-management/document-cookie.html | getting cookie for a cookie-averse document returns empty string, setting does nothing - expected FAIL That looks fun, too :(
The Linux test failures were caused because of whenWindowLoaded. Just waiting for the delayed startup event should be enough.
Flags: needinfo?(ehsan)
(In reply to Josh Matthews [:jdm] from comment #28) > >TEST-UNEXPECTED-PASS | /html/dom/documents/resource-metadata-management/document-cookie.html | getting cookie for a cookie-averse document returns empty string, setting does nothing - expected FAIL > > That looks fun, too :( According to the spec, these kinds of documents are cookie averse, and should not accept a cookie. With my patch, CreateDummyChannelForCookies returns null since aCodebaseURI is null, which makes the SetCookie function fail. I will remove the expected failure annotation from this test.
(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #30) > (In reply to Josh Matthews [:jdm] from comment #28) > > >TEST-UNEXPECTED-PASS | /html/dom/documents/resource-metadata-management/document-cookie.html | getting cookie for a cookie-averse document returns empty string, setting does nothing - expected FAIL > > > > That looks fun, too :( > > According to the spec, these kinds of documents are cookie averse, and > should not accept a cookie. With my patch, CreateDummyChannelForCookies > returns null since aCodebaseURI is null, which makes the SetCookie function > fail. I will remove the expected failure annotation from this test. Does this sound good?
Flags: needinfo?(josh)
Oh heh, I didn't notice the unexpected-pass bit. Yep!
Flags: needinfo?(josh)
Depends on: 1128311
Depends on: 1166066
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: