786407 - "UninstallString" for natively installed web apps should be quoted properly

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect, P2)

Description

[Jesper Hansen]

WebApps cannot be uninstalled if the path to webapp-uninstaller.exe contains spaces such as:
C:\Users\Jesper Staun Hansen\AppData\Roaming\http;m.lanyrd.com\uninstall\webapp-uninstaller.exe


Easily confirmed as navigating to HKEY_USERS\S-1-5-21-3528198288-4018989589-2019204228-1000\Software\Microsoft\Windows\CurrentVersion\Uninstall\http://m.lanyrd.com
then changing UninstallString to place quotes around the string fixes the issue.

There could possible be issues with the other path strings as well, but I do not know how these strings are used.

Comment 1

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

This is probably a one-line fix.  We'll want to change the line [1] to read:
  subKey.writeStringValue("UninstallString",  "\"" + this.uninstallerFile.path + "\"");

I'm not sure, but it probably makes sense to do the same thing for the `InstallLocation` key.

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/webapps/WebappsInstaller.jsm?rev=7e21e4ace8b7#375

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Tim: is this something you can take on?

Comment 3

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch

I haven't built or tested this patch, but this should fix the issue.

I'll post back when I've tested.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 656591 [details] [diff] [review]
Patch

Thanks for looking into this Tim

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 656591 [details] [diff] [review]
Patch

Actually I'm not able to reproduce this bug.

I tested our uninstaller by moving an installation of Favimon to "C:\path with spaces".  I modified the registry entries to point to the new location, but did _not_ add quotes.  I ran the uninstaller and it worked fine.

Additionally, tons of other apps have their uninstallers at locations with spaces in them (especially the "C:\Program Files[...]" variety) and none of them put quotes in their `UninstallString` values.

Jesper, can you provide more precise steps to reproduce?  Or can someone else verify that this is indeed an issue?

Thanks!

Comment 6

[Jesper Hansen]

1. https://dl.dropbox.com/u/72157/Webapp.html#details7
2. Install it
3. Windows control panel (Kontrolpanel)
4. Remove programs (Fjern eller rediger et program) - http://i.imgur.com/yFPJh.png
5. Find QR Code Generator in list
6. Remove (Fjern)
7. Dialog appear with text that removing failed - http://i.imgur.com/wka4s.png

Additional steps to confirm missing quotes:
8. open regedit
9. navigate to HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Uninstall\http://classictutorials.com - http://i.imgur.com/hico7.png
10. Edit UninstallString to have quotes around the entire string - http://i.imgur.com/Ly2TD.png
11. Try to remove again from step 6
12. Removal dialog now appears - http://i.imgur.com/RCMkl.png

If it's not the spaces then I have no idea what it is :-)

Comment 7

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

I created an account on my machine called "Jesper Staun Hansen," installed the "QR Code Generator" app from the dropbox link provided, ran it, then uninstalled it.  I didn't see this issue reproduce so I have to close it as "Works for me."  Please reopen if more information becomes available.  Thanks!

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

A similar issue was recently discussed in bug 868746.

In case users ever copy+paste the UninstallString into a terminal as part of a social engineering attack, we should quote the UninstallString.

Comment 9

[Marco Castelluccio [:marco]]

Could we land your patch (after unbitrotting)?

Comment 10

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Sure, that sounds good

Comment 11

[Marco Castelluccio [:marco]]

Attachment: quite_uninstall_string

I've just updated Tim's patch and it was already reviewed by Felipe. Re-requesting review just to be safe.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/5f0e6a6812e7

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5f0e6a6812e7