Install countdown should respect "security.dialog_enable_delay"

VERIFIED FIXED in mozilla1.8.1beta1

Status

()

--
enhancement
VERIFIED FIXED
15 years ago
11 years ago

People

(Reporter: whimboo, Assigned: zeniko)

Tracking

({verified1.8.1})

1.8 Branch
mozilla1.8.1beta1
verified1.8.1
Points:
---
Bug Flags:
blocking1.8b4 -
blocking1.8b5 -
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

We don't have a chance to circumvent the counter on start of an extension
install. This behavior is not the best for everyone. When i have to install
different extensions during tests it's very anoying to wait this 3 seconds till
the installation could be started. Within SeaMonkey it can be deactiviated
easily by setting following pref: pref("security.dialog_enable_delay", 0); 
But this shoudn't work with Firefox because we have a hardcoded interval within
file xpinstallConfirm.js:

  var XPInstallConfirm = 
  { 
    _installCountdown: 2,
    _installCountdownInterval: -1,
    _param: null
  };

This should be changed to the same behavior within SeaMonkey.

Comment 1

15 years ago
> We don't have a chance to circumvent the counter on start of an extension
> install. This behavior is not the best for everyone.

If you remove the delay, you are making yourself vulnerable to a remote code
execution security hole.  Are you sure you want to do that?  You might be able
to get away with a shorter delay (e.g. 1000ms or 500ms), but removing the delay
makes you vulnerable.
This was an example how we should make this configurable by the user. Which
value everybody want's to use is a decission which can only be come from
himself. If I want to use 0ms than I should do it with all risks. 

This part isn't the main issue of this bug report. Instead we should concentrate
to change the code to use the same behavior like SeaMonkey.

Comment 3

15 years ago
@Jesse
vulnerable? You have to confirm installation. So it's not interesting, if you
wait 0, 500, 1000, 2000 msec. The user has to confirm.

Users like me, who install >5 addons have to wait and wait and wait.

the pref will not work for SeaMonkey too
I use at the moment rv:1.8a2 Gecko/20040615. But several versions before have
the same behaviour

Comment 4

15 years ago
Bug 162020 explains why a setting of 0ms is unambiguously a security hole. 
Finding a good delay amount that works for everyone (500ms, 1000ms, 2000ms, etc)
would require experiments.

Comment 5

15 years ago
(In reply to comment #4)
> Bug 162020 explains why a setting of 0ms is unambiguously a security hole. 
> Finding a good delay amount that works for everyone (500ms, 1000ms, 2000ms, etc)
> would require experiments.

I disagree. Sure, shipping Mozilla/Fx with a 0ms delay would be *NOT* good.
However, this bug is about allowing the user to set how long he wants to wait.
Further, this bug is about re-introducing a missing feature, which is still
implemented in SM.
Depends on: 162020
Within bug 162020 the pref "security.dialog_enable_delay" was mentioned to make
the delay configurable over a pref entry. This is working perfectly for
SeaMonkey but not for Firefox.

=> Updated summary.
Summary: Countdown for installer should be configurable over pref → Countdown for installer is not configurable over pref "security.dialog_enable_delay"
No longer depends on: 162020
The pref mentioned in bug 162020 works for seamonkey because that was a
seamonkey bug. The firefox version of bug 162020 was bug 236056 and was not
written to use a pref. The pref might be nice, but it's an enhancement not a
bug. (likewise, putting the countdown in the button would be a seamonkey
enhancement, not a bug. Different styles for different folks.)
Severity: normal → enhancement
Summary: Countdown for installer is not configurable over pref "security.dialog_enable_delay" → Install countdown should respect "security.dialog_enable_delay"

Comment 8

15 years ago
*** Bug 270295 has been marked as a duplicate of this bug. ***

Comment 9

15 years ago
And why not rewrite it, so the user must check first some checkbox "Yes, I want 
to install this" and then it will be possible to click install. No delay here 
and no remote execution. Just one extra click at different place than the 
install button ...

Dialog would look like:

|----------------------------|
|Do you want to install .... |
|----------------------------|
| [ ] Yes, install extension |
|                            |
|    [Install] [Cancel]      |
|----------------------------|

Also why not spawn the dialog (or at least the install button) outside of mouse 
cursor?
(In reply to comment #9)

Instructions:

 - to play the game, hit the target repeatedly, as it moves around the screen.
 - you will need to press alt-i every time to reload your inventory.

Click = checkbox, alt-i = install.  Circumvented.  Just my opinion, I don't
think this would be better, personally...

-[Unknown]

Comment 11

14 years ago
Created attachment 173421 [details] [diff] [review]
version 1.0

This patch sets countdown based on security.dialog_enable_delay preference.
Attachment #173421 - Flags: review?(bugs)

Comment 12

14 years ago
Re: games that trick you to click.  Isn't this why there is already a whitelist?
 I don't see the harm in a hidden pref that allows power users can set the delay
- yes even a delay of 0ms.  

(In reply to comment #4)
> Bug 162020 explains why a setting of 0ms is unambiguously a security hole. 
> Finding a good delay amount that works for everyone (500ms, 1000ms, 2000ms, etc)
> would require experiments.

Comment 13

14 years ago
*** Bug 307075 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Flags: blocking1.8b4?

Updated

14 years ago
Flags: blocking1.8b5?
Flags: blocking1.8b4?
Flags: blocking1.8b4-

Comment 14

14 years ago
Not a regression over 1.0 and not serious enough for us to stop the release and
it hasn't even gotten into the trunk for baking yet. If this came in before the
first beta we may have considered it but too late now. Try to get it into the
trunk for the next release train.
Flags: blocking1.8b5? → blocking1.8b5-

Comment 15

14 years ago
Comment on attachment 173421 [details] [diff] [review]
version 1.0

This patch has bitrotted
Attachment #173421 - Attachment is obsolete: true
Attachment #173421 - Flags: review?(bugs)

Comment 16

13 years ago
Doesn't this bug also apply to thunderbird? Or is it separate code?
(Assignee)

Comment 17

13 years ago
Created attachment 213329 [details] [diff] [review]
version 1.5
Attachment #213329 - Flags: review?(mconnor)
(Assignee)

Updated

13 years ago
Flags: blocking-firefox2?
Comment on attachment 213329 [details] [diff] [review]
version 1.5


>+  var _installCountdownLength = 5;
>+  try {
>+    var prefs = Components.classes["@mozilla.org/preferences-service;1"]
>+                .getService(Components.interfaces.nsIPrefBranch);

nit: align periods

>+  else {
>+    okButton.label = bundle.getString("installButtonLabel");
>+  }

no brackets around single lines
Attachment #213329 - Flags: review?(mconnor)
Attachment #213329 - Flags: review+
Attachment #213329 - Flags: approval-branch-1.8.1+
not a blocker though
Flags: blocking-firefox2? → blocking-firefox2-
(Assignee)

Comment 20

13 years ago
Created attachment 223804 [details] [diff] [review]
version 1.5 (nits fixed)
(Assignee)

Comment 21

13 years ago
(In reply to comment #18)
> no brackets around single lines

Looks quite funny where there are brackets around the if-clause but not around the else-clause. Where's the style guide when you need it? ;)
Assignee: bugs → zeniko
Whiteboard: [checkin needed]
mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.js 	1.5.2.3
mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.js 	1.8
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → Firefox 2 beta1
Version: unspecified → 2.0 Branch
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.