1120673 - Fx 35 installer crashes on XP x86 SP3 at the end (creating shortcuts) if the xp firewall service is stopped

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[al_9x]

reproducible in a vm

AppName: setup.exe	 AppVer: 1.0.0.0	 ModName: msvcrt.dll
ModVer: 7.0.2600.5512	 Offset: 00032a16

Comment 1

[Robert Strong (they/them - no direct email)]

Unable to reproduce with WinXP SP2 on a VM (had WinXP SP 2 handy).

Kamil, can you try to reproduce? Thanks!

Comment 2

[Robert Strong (they/them - no direct email)]

Also tried installing on top of an existing install and was unable to reproduce on WinXP SP2

Comment 3

[Kamil Jozwiak [:kjozwiak]]

Used the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0-candidates/build3/

Checked on the following OS's:
- Win XP Pro x86 SP3 - PASSED (ran through it two times)
- Win XP Pro x86 SP2 - PASSED (ran through it two times)
- Win XP Pro x64 SP2 - PASSED (ran through it two times)

Looks like it's working for me. I also launched fx a few times via the shortcut and everything worked without any issues. Couldn't reproduce any crashes :(

Comment 4

[Robert Strong (they/them - no direct email)]

Reporter, does this reproduce everytime on your system?

Comment 5

[al_9x]

to repro, stop the windows firewall service
_________________________________

call stack from drwtsn32.log

msvcrt!wscanf+0x750
msvcrt!printf+0x35
liteFirewallW+0x1292
liteFirewallW!AddRule+0x4c
setup+0x21d8
setup+0x13ba
setup+0x13ba
__________________________________

setup should not crash if firewall is off, but why are you adding incoming firewall rules in the first place? it certainly is not needed fro browsing.

Comment 6

[Robert Strong (they/them - no direct email)]

Thanks!

Comment 7

[al_9x]

you didn't answer:

why are you adding incoming firewall rules in the first place? it certainly is not needed for browsing.

Comment 8

[Kamil Jozwiak [:kjozwiak]]

Added STR (reproduced twice):

1) Download http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0-candidates/build3/
2) Disable "Windows Firewall/Internet Connection Sharing (ICS)" under Services
3) Install the above build, will receive the crash during the "Create shortcut..." process

Comment 9

[Robert Strong (they/them - no direct email)]

Sorry, didn't see the question. It was added to support bug 1054959. To disable it you can change the rule to a deny rule.

Comment 10

[Kamil Jozwiak [:kjozwiak]]

Robert, here's more information if you need it:

Turning off "Windows Firewall/Internet Connection Sharing (ICS)"

- Win XP SP2 x86 - Reproduced
- Win XP SP3 x86 - Reproduced
- Win XP SP2 x64 - Reproduced

Turning off "Windows Firewall"

- Win 7 Pro SP1 x86 - Couldn't reproduce the crash with the service mentioned above turned off (x2)
- Win 7 Pro SP1 x64 - Couldn't reproduce the crash with the service mentioned above turned off (x2)

- Win 8 x86 - Couldn't reproduce the crash with the service mentioned above turned off (x2)
- Win 8.1 x86 - Couldn't reproduce the crash with the service mentioned above turned off (x2)

- Win 8 x64 - Couldn't reproduce the crash with the service mentioned above turned off (x2)
- Win 8.1 x64 - Couldn't reproduce the crash with the service mentioned above turned off (x2)

I'll check Vista once I have them installed tomorrow.. Haven't added Vista since upgrading to VM Fusion 7. They also take the longest to install :/

Comment 11

[Robert Strong (they/them - no direct email)]

Thanks Kamil! I have Vista VMs so no worries.

Comment 12

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Comment 13

[Robert Strong (they/them - no direct email)]

[Tracking Requested - why for this release]:

Comment 14

[Robert Strong (they/them - no direct email)]

Attachment: patch with comments and register preservation

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 8549368 [details] [diff] [review]
patch with comments and register preservation

Tested successfully on Win XP, Win Vista, Win 7, and Win 8.1 with and without the service and with an admin and non-admin accounts.

Comment 16

[Robert Strong (they/them - no direct email)]

Comment on attachment 8549368 [details] [diff] [review]
patch with comments and register preservation

Brian, I wanted to run the register preservation changes by you. Thanks!

Comment 17

[Robert Strong (they/them - no direct email)]

[Tracking Requested - why for this release]:
On Win XP when the Firewall service isn't running the installer crashes towards the end after creating shortcuts. If there is a Firefox 35 point release I'd like to get this in.

Comment 18

[Brian R. Bondy [:bbondy]]

Comment on attachment 8549368 [details] [diff] [review]
patch with comments and register preservation

Review of attachment 8549368 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me Robert!  One nit inline.

::: browser/installer/windows/nsis/shared.nsh
@@ +1570,5 @@
> +  Push $R7
> +  Push $R6
> +  Push "false"
> +
> +  System::Call 'advapi32::OpenSCManagerW(n, n, i ${SC_MANAGER_ALL_ACCESS}) i.R6'

I'd ask for less access here if possible.  Could fail for some account setups.

Comment 19

[Stephen A Pohl [:spohl]]

Hey Robert, I'm awfully unfamiliar with nsis. Did you want me to review everything, or was there something specific that you wanted my feedback on? I'm happy to go over the patch, with the caveat that it may take me some time to understand it well enough to give it my r+. If this is urgent I'm happy to defer to Brian's review. Just let me know. Thanks!

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

We're still collecting potential drivers & ride along issues, tracking this as a potential ride along.

Comment 21

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

Since jsavage (who normally writes english articles) is on PTO, ideato, madalina and I wrote a SUMO KB article about this:

https://support.mozilla.org/en-US/kb/firefox-installer-crashes-windows-xp

Comment 22

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-central with sheriff approval
https://hg.mozilla.org/mozilla-central/rev/89a190592267

Comment 23

[Robert Strong (they/them - no direct email)]

Comment on attachment 8549368 [details] [diff] [review]
patch with comments and register preservation

Approval Request Comment
[Feature/regressing bug #]: bug 1090535
[User impact if declined]: Installing on Win XP when the Firewall service is not running will crash the installer
[Describe test coverage new/current, TBPL]: Manually tested on Windows with and without the Firewall service running.
[Risks and why]: minimal due to lots of manual testing.
[String/UUID change made/needed]: None.

Note: I'd also like to get this into release if there is a point release

Comment 24

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/374a9adf2786
https://hg.mozilla.org/releases/mozilla-beta/rev/0f43b4df53bb

Comment 25

[Robert Strong (they/them - no direct email)]

Comment on attachment 8549368 [details] [diff] [review]
patch with comments and register preservation

Approval Request Comment
[Feature/regressing bug #]: bug 1090535
[User impact if declined]: Installing on Win XP when the Firewall service is not running will crash the installer
[Describe test coverage new/current, TBPL]: Manually tested on Windows with and without the Firewall service running.
[Risks and why]: minimal due to lots of manual testing.
[String/UUID change made/needed]: None.

Comment 26

[Kamil Jozwiak [:kjozwiak]]

Attachment: installWithFirewallEnabled.png

Test Cases used for verification:

Test Case #1:

* Disabled "Windows Firewall/Internet Connection Sharing (ICS)"
* Installed build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-21-03-02-03-mozilla-central/
* re-launched fx several times and ensured there wasn't any crashes
* re-enabled "Windows Firewall/Internet Connection Sharing (ICS)" and launched fx several times
** received the Firewall warning and selected "unblock"
** ensured that Nightly appeared under "Exceptions" list under "Windows Firewall"
* Disabled "Windows Firewall/Internet Connection Sharing (ICS)" once again and opened fx several times
* re-enabled "Windows Firewall/Internet Connection Sharing (ICS)" and launched fx several times

Test Case #2:

* installed an older version of Nightly
* once installed, disabled "Windows Firewall/Internet Connection Sharing (ICS)"
* proceeded with Nightly update with the firewall disabled
* ensured that Nightly updates without any issues or crashes and fx launches without any problems
* enabled "Windows Firewall/Internet Connection Sharing (ICS)" and launched FX several times

Went through the above test cases on the following OS's:

- Windows XP Pro x86 SP2 - PASSED
- Windows XP Pro x86 SP3 - PASSED
- Windows XP Pro x64 SP2 - PASSED

One thing that I noticed while going through this:

* Nightly appears as "Nightly (C:\Program Files\Nightly)" under the exception list when installing fx while the firewall service is enabled
** Check installWithFirewallEnabled image
* Nightly appears as "Nightly" under the exception list when installing fx while the firewall is enabled via the "Unblock" prompt
** Check 

Robert, this doesn't seem like a problem but just making sure this doesn't lead to anything more serious?

Comment 27

[Kamil Jozwiak [:kjozwiak]]

Attachment: installWithoutFirewallEnabled.png

Comment 28

[Robert Strong (they/them - no direct email)]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #26)
> Created attachment 8552575 [details]
> installWithFirewallEnabled.png
> 
> Test Cases used for verification:
> 
> Test Case #1:
> 
> * Disabled "Windows Firewall/Internet Connection Sharing (ICS)"
> * Installed build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-21-03-02-03-
> mozilla-central/
> * re-launched fx several times and ensured there wasn't any crashes
> * re-enabled "Windows Firewall/Internet Connection Sharing (ICS)" and
> launched fx several times
> ** received the Firewall warning and selected "unblock"
> ** ensured that Nightly appeared under "Exceptions" list under "Windows
> Firewall"
> * Disabled "Windows Firewall/Internet Connection Sharing (ICS)" once again
> and opened fx several times
> * re-enabled "Windows Firewall/Internet Connection Sharing (ICS)" and
> launched fx several times
> 
> Test Case #2:
> 
> * installed an older version of Nightly
> * once installed, disabled "Windows Firewall/Internet Connection Sharing
> (ICS)"
> * proceeded with Nightly update with the firewall disabled
> * ensured that Nightly updates without any issues or crashes and fx launches
> without any problems
> * enabled "Windows Firewall/Internet Connection Sharing (ICS)" and launched
> FX several times
> 
> Went through the above test cases on the following OS's:
> 
> - Windows XP Pro x86 SP2 - PASSED
> - Windows XP Pro x86 SP3 - PASSED
> - Windows XP Pro x64 SP2 - PASSED
> 
> One thing that I noticed while going through this:
> 
> * Nightly appears as "Nightly (C:\Program Files\Nightly)" under the
> exception list when installing fx while the firewall service is enabled
> ** Check installWithFirewallEnabled image
> * Nightly appears as "Nightly" under the exception list when installing fx
> while the firewall is enabled via the "Unblock" prompt
> ** Check 
> 
> Robert, this doesn't seem like a problem but just making sure this doesn't
> lead to anything more serious?
That's intentional so we are able to distinguish between user created exceptions and exceptions that we create.

Thanks again!

Comment 29

[Kamil Jozwiak [:kjozwiak]]

Awesome, thanks Robert!

Comment 30

[Kamil Jozwiak [:kjozwiak]]

Used the following build with the steps from comment #26:
* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-18-00-40-06-mozilla-aurora/
* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-21-00-40-11-mozilla-aurora/

- Ensured that both test cases from comment #26 worked without any issues
- Ensured that aurora appeared as "FirefoxDeveloperEdition" under the exception list of Windows Firewall when manually added
- Ensured taht aurora appeared as "FirefoxDeveloperEdition (C:\Program Files\Firefox Developer Edition)" under the exception list of Windows Firewall when automatically added

OS's:

- Windows XP Pro x86 SP2 - PASSED
- Windows XP Pro x86 SP3 - PASSED
- Windows XP Pro x64 SP2 - PASSED

Comment 31

[Kamil Jozwiak [:kjozwiak]]

Used the following build with the steps from comment #26:
* http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/36.0b1-candidates/build2/win32/en-US/
* http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/36.0b2-candidates/build1/win32/en-US/

- Ensured that both test cases from comment #26 worked without any issues
- Ensured that beta appeared as "Firefox" under the exception list of Windows Firewall when manually added
- Ensured that beta appeared as "Firefox (C:\Program Files\Mozilla Firefox)" under the exception list of Windows Firewall when automatically added

OS's:

- Windows XP Pro x86 SP2 - PASSED
- Windows XP Pro x86 SP3 - PASSED
- Windows XP Pro x64 SP2 - PASSED

Comment 32

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-release/rev/424c99d64c4b

Comment 33

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0.1-candidates/build1/win32/en-US/

- ensured that the fx firewall entry is being added into the exceptions list when installing
- launched fx several times and ensured that it opens without any crashes
- ensured that fx installs without any issues/crashes when the firewall service has been disabled
- launched fx several times and ensured that it opens when the firewall service has been disabled

One thing I noticed is that users don't get the firewall warning if they re-enable the firewall service and launch FX (if it hasn't already been added into the exceptions). Talked to Robert and it appears like it's MS code that decides when to prompt and when not to prompt users.

Os's used:

- Windows XP Pro x86 SP2 - PASSED
- Windows XP Pro x86 SP3 - PASSED
- Windows XP Pro x64 SP2 - PASSED