245737 - Install countdown should respect "security.dialog_enable_delay"

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

[Jesse Ruderman]

> 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.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

[Frank Widmaier]

@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

[Jesse Ruderman]

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

[Alexander Skwar]

(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.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 7

[Daniel Veditz [:dveditz]]

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.)

Comment 8

[OstGote!]

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

Comment 9

[Martin Petricek]

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?

Comment 10

[Unknown W. Brackets]

(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

[Richard Klein]

Attachment: version 1.0

This patch sets countdown based on security.dialog_enable_delay preference.

Comment 12

[Devon Jensen]

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

[Adam Guthrie]

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

Comment 14

[Scott MacGregor]

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.

Comment 15

[Richard Klein]

Comment on attachment 173421 [details] [diff] [review]
version 1.0

This patch has bitrotted

Comment 16

[Eyal Rozenberg]

Doesn't this bug also apply to thunderbird? Or is it separate code?

Comment 17

[Simon Bünzli]

Attachment: version 1.5

Comment 18

[Mike Connor [:mconnor]]

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

Comment 19

[Mike Connor [:mconnor]]

not a blocker though

Comment 20

[Simon Bünzli]

Attachment: version 1.5 (nits fixed)

Comment 21

[Simon Bünzli]

(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? ;)

Comment 22

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.js 	1.5.2.3
mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.js 	1.8