Closed
Bug 388887
Opened 17 years ago
Closed 16 years ago
Privileges automatically enabled with fresh profile and "default browser" dialog
Categories
(Core :: Security, defect, P5)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: dholbert, Unassigned)
References
Details
(Whiteboard: [sg:moderate] fixed by 206691)
Attachments
(4 files)
Posting test case in a sec. Requirements:
- Testcase must be saved locally. (not viewed on the web)
- Use a fresh profile
- Specify testcase filename on command line, so it opens as first page.
- Run a version of Trunk that's <b>not</b> your default browser. (The automatic "Make me your default browser?" dialog is needed for this testcase to work.)
e.g.
mkdir foo;
/path/to/firefox -profile foo -no-remote /path/to/testcase.html
Observed behavior:
- An empty Ok/Cancel dialog box pops up behind Default Browser dialog.
- Clicking Ok, Cancel, or even closing this dialog box, will all enable privileges.
Expected behavior:
- Allow/Deny Privileges dialog box should appear in place of the empty Ok/Cancel dialog.
Even better (although not security-sensitive) would be for the Allow/Deny Privileges box to appear *in front* of the Default Browser box, because you can't click anything in Default Browser dialog until you resolve the Allow/Deny Privileges box.
Reporter | ||
Comment 1•17 years ago
|
||
Comment 2•17 years ago
|
||
Mmmm.... fun and tangy event loop stuff. :(
I would have thought that pushing and popping null js contexts properly during the event loop would make this better... guess not.
Reporter | ||
Comment 3•17 years ago
|
||
One caveat when reproducing bug on Windows -- to get the Default Browser box to appear, it looks like *no* version of Firefox can be your default browser. You can temporarily set IE 7 as your default browser via the "Programs" tab of IE7's "Internet Options" dialog.
Also: When trying the testcase in Firefox 2, the Firefox window seems to lock up, without displaying any dialogs. It doesn't seem to enable the privileges in Firefox 2, though.
Reporter | ||
Comment 4•17 years ago
|
||
Here's a smaller testcase.
Reporter | ||
Comment 5•17 years ago
|
||
Save both testcases by right-clicking link and choosing "save-as" or using "view-source", rather than using File|Save.
Reporter | ||
Updated•17 years ago
|
Summary: Privileges automatically enabled on fresh profile and "default browser" dialog → Privileges automatically enabled with fresh profile and "default browser" dialog
Reporter | ||
Updated•17 years ago
|
Flags: blocking1.9?
So this doesn't seem like it will happen very often so it isn't a huge problem.
What I do think we should do though is make sure that we default to *not* granting access when we end up with this broken dialog. Could we make sure that happens more easily? Possibly that could be done in the call to the dialog, or by chaning the dialog itself.
Flags: blocking1.9? → blocking1.9+
Comment 7•17 years ago
|
||
I can't seem to reproduce this with a debug Linux trunk build, for what it's worth... :(
Reporter | ||
Comment 8•17 years ago
|
||
(In reply to comment #7)
> I can't seem to reproduce this with a debug Linux trunk build, for what it's
> worth... :(
>
Hmm... I just re-tested this with my up-to-date debug Linux trunk build (checked out this morning), as well as with the latest nightly, and I got these results:
- My debug trunk build: I still get behavior as described in comment 0.
- Latest Nightly build: The dialog boxes switch roles -- the "Privileges" one shows up in full, and the default-browser bugging one shows up as blank. (and no matter what button I click on the blank dialog, the nightly build of firefox is set as my default browser.)
Here are my .mozconfig settings, for what it's worth:
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj
mk_add_options MOZ_CO_PROJECT=browser
ac_add_options --enable-application=browser
ac_add_options --enable-debug --disable-optimize
ac_add_options --enable-extensions=default,layout-debug
ac_add_options --disable-libxul
ac_add_options --disable-airbag
ac_cv_visibility_pragma=no
export CFLAGS="-fmessage-length=0"
export CPPFLAGS="-fmessage-length=0"
Comment 9•17 years ago
|
||
Could be a matter of GTK version or something again...
But really, this should not be happening. We now push the proper null principals everywhere. So what's going on?
Reporter | ||
Comment 10•17 years ago
|
||
(In reply to comment #9)
> Could be a matter of GTK version or something again...
Hmm... it's possible that the bug is masked in your particular version of GTK, but it's definitely not a GTK-*dependent* issue, because I can reproduce it in Windows. (Unless we use some GTK library on Windows, but I'd imagine that we don't)
I just now tested the latest nightly trunk build on a WinXP box, and the bug reproduced with no trouble on both testcases. I got these results:
- Verbose testcase: the Privileges dialog was blank (and auto-accepted)
- Small testcase: the "default browser" dialog was blank (and auto-accepted)
Based on this, I'd guess that there's a race condition between the two dialogs, and whichever dialog we request *latest* becomes blank and auto-accepted.
(I'm guessing that the default-browser dialog always pops up at about the same time, as part of the launch process, whereas the privilege-dialog's timing depends on how long it takes us to render the page content.)
Comment 11•17 years ago
|
||
Could be, yeah...
Are there any errors in the error console? Make sure you enable reporting of chrome errors.
Reporter | ||
Comment 12•17 years ago
|
||
I'm getting this assertion around the time the browser window appears, and I think it's probably related:
###!!! ASSERTION: CachedChromeStream doesn't receive data: 'Not Reached', file /scratch/work/builds/trunk.07-07-31.09-38/mozilla/content/xul/document/src/nsXULDocument.cpp, line 4329
mxr link: http://mxr.mozilla.org/seamonkey/source/content/xul/document/src/nsXULDocument.cpp#4329
Comment 13•17 years ago
|
||
What's the stack to that assert?
Reporter | ||
Comment 14•17 years ago
|
||
(In reply to comment #11)
> Are there any errors in the error console? Make sure you enable reporting of
> chrome errors.
No errors in error console, with chrome error reporting turned on.
Reporter | ||
Comment 15•17 years ago
|
||
(In reply to comment #13)
> What's the stack to that assert?
It's weird...
----
#0 JS_AddRoot (cx=0xbfeb229c, rp=0xb7d3be76)
at /scratch/work/builds/trunk.07-07-31.09-38/mozilla/js/src/jsapi.c:1745
#1 0xbfeb1ea8 in ?? ()
#2 0xbfeb229c in ?? ()
#3 0xb7d3be76 in JS_PrintTraceThingInfo (
buf=0xb73a9560 "\207(���\225:��\225:��\225:��\225:��\225:��\225:��\225:��\225:�",
bufsize=3084375913, trc=0xbfeb1ea8, thing=0x10e9, kind=3084399906, details=3)
at /scratch/work/builds/trunk.07-07-31.09-38/mozilla/js/src/jsapi.c:2017
Backtrace stopped: frame did not save the PC
-----
Not sure why I get that weird backtrace for this assert... I get a nicer one from this earlier assertion:
###!!! ASSERTION: nsNSSComponent relies on profile manager to wait for synchronous shutdown of all network activity: 'mIsNetworkDown', file /scratch/work/builds/trunk.07-07-31.09-38/mozilla/security/manager/ssl/src/nsNSSComponent.cpp, line 2305
Reporter | ||
Comment 16•17 years ago
|
||
Actually, ignore that last backtrace -- it looks like it gives me a different backtrace every time I run it. (maybe it's reporting the backtrace from the wrong thread?)
Reporter | ||
Comment 17•17 years ago
|
||
Here's a backtrace for the assertion mentioned in comments 12,13,15,16.
(I think I wasn't able to get one before because the assertion was in a library whose symbols hadn't successfully loaded in GDB, or something like that.)
Ignore backtrace levels 0 and 1 -- those are from trapping into GDB. Level 2 is the actual NS_NOTREACHED.
Comment 18•17 years ago
|
||
Hmm. I wonder where we end up with a CachedChromeStreamListener as the listener for a DocumentOpenInfo.... that seems like the real bug here. I bet the fact that the same XUL file implements both dialogs is important here. And the fresh profile is probably key because you want to avoid fastload....
Comment 19•17 years ago
|
||
So in particular, when nsChromeProtocolHandler::NewChannel is called for whichever dialog comes up second we end up with the document not in the XUL prototype cache (beacuse it's still loading as a result of the first dialog coming up) so we start an actual load of the document (the second such load; one is already in progress).
Then when the second load actually goes to create its XUL document via StartDocumentLoad(), we discover that the document is now in the cache and create a CachedChromeStreamListener instead of a parser to handle the load... but our load isn't coming from the cache, leading to this assertion.
Basically, the code seems to assume that if the document is not in the cache at NewChannel() time it also won't be there at StartDocumentLoad() time. Which is not a great assumption, really. It would be OK if NewChannel() created the prototype and put it in the cache, but it doesn't seem to do that.
The rest of the bug is more of a problem. The issue is that nsDialogParamBlock::nsDialogParamBlock does:
46 for(PRInt32 i = 0; i < kNumInts; i++)
47 mInt[i] = 0;
and the value of buttonPressed is gotten as:
353 block->GetInt(eButtonPressed, buttonPressed);
in nsPromptService::ConfirmEx. Since apparently no one fills in any of the ints on the dialog param block in this case, we get back 0, which means OK. Somewhere in this code, imo, we should probably preset that value to 1 on the dialog param block. Possibly in ConfirmEx, but maybe somewhere deeper. biesi, any ideas?
Priority: -- → P5
Reporter | ||
Comment 20•17 years ago
|
||
fwiw, I'm still seeing this bug using a fresh trunk build from today:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007110715 Minefield/3.0b2pre
Updated•17 years ago
|
Flags: wanted1.9.0.x+
Flags: tracking1.9+
Flags: blocking1.9-
Comment 21•16 years ago
|
||
Marking sg:high since it allows potential machine ownership with major mitigations.
Whiteboard: [sg:high]
Updated•16 years ago
|
Assignee: dveditz → nobody
Comment 22•16 years ago
|
||
Has anyone had any flashes of inspiration as to how to fix this one?
Reporter | ||
Comment 23•16 years ago
|
||
FWIW: In mozilla-central, it looks like the dialogs have switched position... The privileges dialog now appears in full and behaves normally, and the "default browser" dialog is the blank (and auto-accepted) one. Here's a screenshot of the new behavior.
I tried this 5 times (with a fresh profile and copy of firefox each time, to make sure to spawn the right dialogs), and I got this new behavior repeatably. (Then I tried it with an old build just to make sure I wasn't doing something different, and I got the old/scary behavior.)
So, assuming that the behavior has definitively changed, this bug still isn't fixed -- it's just changed slightly so as to be a little less scary. (but perhaps more annoying -- it changes my default browser without any clear notification or any way to cancel)
The new build I tested (shows blank default-browser dialog):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090122 Minefield/3.2a1pre
The old build I tested (shows blank enable-privs dialog):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007072504 Minefield/3.0a7pre
Reporter | ||
Comment 24•16 years ago
|
||
IIRC, I've seen similar blank-dialog-when-Firefox-starts issues with other dialog combinations -- e.g. default browser + master-password dialog, if you start Firefox with a page that uses one of your saved passwords. I think those issues are probably the same underlying bug as this one.
Comment 25•16 years ago
|
||
Comment 19 describes the way (or rather some alternative ways) to fix the "automatically enabled" part of this bug... It won't help the blank dialog thing, which basically needs some XUL help. There are various options there, including explicitly flagging on the channel where it's coming from, and having the XUL document check that.
Comment 26•16 years ago
|
||
So, to trigger the assertion, you need a) xul cache enabled and b) to open an uncached but cacheable document in two windows near-simultaneously?
Comment 27•16 years ago
|
||
Interestingly, my latest work for bug 206691 may well end up fixing this bug and also one cause of bug 465998 (another of the causes has since been fixed).
Comment 28•16 years ago
|
||
Only the nsXULDocument.cpp changes from attachment 358702 [details] [diff] [review] are required here.
Comment 29•16 years ago
|
||
Comment 26 is correct.
Comment 30•16 years ago
|
||
Now bug 206691 is fixed, can someone retest with the next 1.9.2a1pre Minefield?
Reporter | ||
Comment 31•16 years ago
|
||
Awesome -- this is WFM now, in the latest nightly! I tested yesterday's nightly, to confirm that it was still broken there, and it was.
BROKEN (behaves as described in comment 23):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090129 Minefield/3.2a1pre
WORKING (neither dialog blank, neither dialog automatically confirms):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090130 Minefield/3.2a1pre
Resolving as WORKSFORME. Neil, feel free to change resolution to FIXED & add dependency on that other bug, if you're sure that's what fixed it.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Comment 32•16 years ago
|
||
OK, so what part(s) of attachment 358846 [details] [diff] [review] do we want on the branches?
Comment 33•16 years ago
|
||
This should now have been fixed for 1.9.1 by changeset f0deff7035c6.
Updated•15 years ago
|
Whiteboard: [sg:moderate] → [sg:moderate] fixed by 206691
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•