Closed Bug 882624 Opened 11 years ago Closed 11 years ago

Remove "URL Protocol" from FirefoxURL key

Categories

(Firefox :: Installer, defect)

x86_64
Windows 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 24

People

(Reporter: emk, Assigned: emk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We have never intended to expose "firefoxurl://" as a URL protocol. It caused a security vulnerability in the past (bug 384384).
"IE.FTP", "IE.HTTP", and "IE.HTTPS" keys don't have the "URL Protocol" named value.
Attached patch patchSplinter Review
Deleting the value explicitly because old installers may set the value.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #761982 - Flags: review?(robert.bugzilla)
Attachment #761982 - Flags: review?(robert.bugzilla) → review?(netzen)
Comment on attachment 761982 [details] [diff] [review]
patch

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

Please wait to implement the nit as there will likely be some more comments.

I won't be able to finish reviewing this until next week because I need to setup some clean registry VMs.
Leaving the review request until then.

As far as I recall this is needed but I'll verify. If we do make changes here, once this lands we should also post a bug for seamonkey.

::: toolkit/mozapps/installer/windows/nsis/common.nsh
@@ +1495,5 @@
>  
>        StrCmp "$R9" "true" +1 +2
>        WriteRegStr SHCTX "$R3\$R5" "URL Protocol" ""
> +      StrCmp "$R9" "delete" +1 +2
> +      DeleteRegValue SHCTX "$R3\$R5" "URL Protocol"

nit: Please add to this comment at the top of this function:
Exch $R9 ; true if a protocol handler
Comment on attachment 761982 [details] [diff] [review]
patch

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

Seems like it's only needed for custom URL protocols. So this looks good.

I tried it on XP, Vista, and Win8 and it works fine.

On Vista and Win8 (where there's a defaults control panel item) I could still set the default protocol handlers for Firefox and Firefox showed up in the list of default handlers for HTTP, HTTPS, and FTP protocols.
Attachment #761982 - Flags: review?(netzen) → review+
Blocks: 883855
https://hg.mozilla.org/mozilla-central/rev/5aea44ac54ce
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Sorry, I forgot to reflect the review nit.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bef58eae5f12
Keywords: checkin-needed
Why was checkin-needed set?
Keywords: checkin-needed
Sorry, I missed the bug number (I meant bug 884301).
Remove "URL Protocol" from FirefoxURL key cause pinned shortcut fail to open in win7/8 jumplist.

This works in FF 23, fail in FF 24.
(In reply to hunreal from comment #10)
> Remove "URL Protocol" from FirefoxURL key cause pinned shortcut fail to open
> in win7/8 jumplist.
> 
> This works in FF 23, fail in FF 24.

hunreal, could you please post a new bug describing this problem? You can use the link at the bottom of the page named Clone this bug.  Please put exact steps on how to pin a shortcut if it is different from normal shortcuts that are autopopulated by the app. Thanks!
Depends on: 911158
The removal of this registry value is causing some problems for some people (see bug 911158).
Emk, are you OK with me reverting this? I think that this bug was originally just a precaution and not an actual attack.
Flags: needinfo?(VYV03354)
OK.
Flags: needinfo?(VYV03354)
Blocks: 921018
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: