Closed Bug 239411 Opened 19 years ago Closed 18 years ago

install delay should stop+reset when Software Installation dialog loses focus


(Toolkit :: Add-ons Manager, defect, P4)

Windows XP





(Reporter: jruderman, Assigned: benjamin)



(4 keywords)


(2 files)

Ben's patch in bug 236056 doesn't guard against a variant of the attack I
alluded to in bug 162020 comment 0:
1. Open a new window.
2. In the original window, pop up an XPI dialog.
3. Convince the user to double-click somewhere in the new window.
4. On the first click, close the new window, letting the XPI dialog show through.

If the dialog starts without focus or loses focus, it needs to disable the
button and start counting down when the dialog gains focus.  This will probably
annoy Linux users whose window managers don't focus modal dialogs, but I don't
see an obvious way around that.
You have to allow scripts to "Raise or lower windows" for this exploit to work well.
Whiteboard: security
jesse, that is an excellent exploit.
Bug 162020 is blocking1.7+, so this should be too.
Flags: blocking0.9?
any thoughts on how we are going to plug this?
Flags: blocking0.9? → blocking0.9+
well, if you want something for 1.7, how about a modal dialog?  
The XPI dialog is already modal.  Are you suggesting making it application-modal?
obviously. ;-)
This exploit is less dangerous since bug 238684 was fixed. The testcase given
doesn't work in recent builds, since XPI installation from a timer is
disallowed. It also doesn't work to use a load listener on the decoy window,
instead of a timer. That's because the XPInstallManager seems to be invoked with
its parent window set to the topmost window, even if invoked from another
window, and the topmost window in this case is the still-loading decoy window.
XPI installation is also disallowed from loading windows.

I'm not saying this is no longer a problem. There may be some way to begin the
installation on the opener window from, say, the decoy window's mousemove
handler in the example. Focus the opener, begin installation, and then return
focus to yourself, or some such thing. I haven't been able to make it work.
There'd be flashing of windows, at best.

In case this exploit can be made to work, I rather like Jesse's suggestion of
stopping the installation countdown while the XPInstall dialog isn't focused.
It's simple, it's elegant, it's sufficient, isn't it?, and it doesn't require
the spanking the widget code would need before it could support
application-modal windows.

Sadly, I can't make that work, either. The XUL change is simple enough but I
can't seem to get blur notifications in the XPInstall dialog unless I first hit
the Cancel button (and drag the mouse off before releasing). See bug 164686.
Flags: blocking1.0+
Flags: blocking0.9-
Flags: blocking0.9+
Whiteboard: security → [sg:fix] security
p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Assignee: bugs → benjamin
Flags: blocking1.8b4+
This does make one UI change: it counts down by tenths of a second, instead of
by whole seconds, because it easy to click back to the install confirmation and
see (2) and not realize that it's counting down again.
Attachment #186601 - Flags: review?(bugs)
If this doesn't cause leaks on trunk it's probably 1.0.x-worthy.
Component: Download Manager → Extension/Theme Manager
Flags: blocking-aviary1.0.6?
Flags: blocking-aviary1.0.5?
1.0.5 just about wrapped up.  we might want this if we do a 1.0.6

Flags: blocking-aviary1.0.5? → blocking-aviary1.0.5-
Whiteboard: [sg:fix] security → [sg:fix] security has-patch, needs-review(BenG)
Whiteboard: [sg:fix] security has-patch, needs-review(BenG) → [sg:fix][no l10n impact] security has-patch, needs-review(BenG)
Comment on attachment 186601 [details] [diff] [review]
Reset the xpinstall counter onblur, rev. 1
Attachment #186601 - Flags: review?(bugs) → review+
Fixed on trunk for 1.8b4
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [sg:fix][no l10n impact] security has-patch, needs-review(BenG) → [sg:fix][no l10n impact] security has-patch
Target Milestone: --- → Firefox1.1
Attachment #186601 - Flags: approval-aviary1.0.7?
Depends on: 302265
Keywords: fixed1.8
Whiteboard: [sg:fix][no l10n impact] security has-patch → [sg:low][no l10n impact] security has-patch
Won't kill us not to fix this, not blocking (but I'll leave the approval request for now). Fixed in 1.5 which will be released before 1.0.8, mainly interested in severe problems on the old branch.
Flags: blocking-aviary1.0.8? → blocking-aviary1.0.8-
Flags: testcase+
Comment on attachment 186601 [details] [diff] [review]
Reset the xpinstall counter onblur, rev. 1

not needed for old branches
Attachment #186601 - Flags: approval-aviary1.0.8? → approval-aviary1.0.8-
Group: security
Flags: in-testsuite+ → in-testsuite?
Product: Firefox → Toolkit
Keywords: csec-spoof, sec-low
Whiteboard: [sg:low][no l10n impact] security has-patch
You need to log in before you can comment on or make changes to this bug.