Closed Bug 1567614 Opened 2 months ago Closed 2 months ago

"Entry Point Not Found" message box when opening Skype for Business URL

Categories

(External Software Affecting Firefox :: Other, defect, P1)

Unspecified
Windows
defect

Tracking

(firefox-esr60 unaffected, firefox-esr6868+ verified, firefox68+ verified, firefox69+ verified, firefox70+ verified)

VERIFIED FIXED
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 68+ verified
firefox68 + verified
firefox69 + verified
firefox70 + verified

People

(Reporter: aklotz, Assigned: aklotz)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(3 files)

I have managed to reproduce the errors mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1564546#c1

STR:

  1. Install Firefox release.
  2. Install Skype for Business from the Windows Store.
  3. Run Firefox. Go to the address bar, type lync15://foo, and press enter
  4. Firefox will display a window asking you to confirm the handler application. Select Skype for Business and click Open link.

Expected:
Skype for Business starts (note that since we don't have accounts, we can't actually log in to do anything with the link)

Actual:
I am then shown an error message box:
Caption: lync.exe - Entry Point Not Found
Message:

The procedure entry point GetCurrentProcessId could not be located in the dynamic link library C:\Program Files\WindowsApps\Microsoft.Office.Desktop.SkypeForBusiness_16051.11727.20244.0_x86__8wekyb3d8bbwe\Office16\RtmMvrSplitter.dll.

The code that starts lync.exe is a ShellExecuteEx call.

While the error message box is up, the lync.exe process continues living. Examining it in Process Explorer, I don't see anything there that differs significantly from a lync.exe process that was started manually.

I'll try doing it with process mitigations set and see how that works.

I have reproduced this via a simple test program!

Indeed, the failure is due to the fact that the process calling ShellExecuteEx has the PreferSystem32Images process mitigation policy set.

I sent an email to the Microsoft discussion list.

Duplicate of this bug: 1567090

According to our Microsoft contacts, process mitigations are inherited by child processes. Those child processes might not like what they are inheriting, as is the case for Skype for Business.

There are two options that I see for resolving this:

  • We already have code in the launcher process that does not directly perform a ShellExecute, but rather asks Explorer to do it on our behalf. I modified my test program to do this, and this does work. OTOH I am concerned about security implications, as based on the code we'd need to be careful about not reintroducing bug 394974.
  • We back out the setting of mitigation policies on the browser process until we can come up with a bulletproof way of launching these URLs.

(In reply to Aaron Klotz [:aklotz] from comment #5)

  • We already have code in the launcher process that does not directly perform a ShellExecute, but rather asks Explorer to do it on our behalf. I modified my test program to do this, and this does work. OTOH I am concerned about security implications, as based on the code we'd need to be careful about not reintroducing bug 394974.

I have figured out a way to make all of this work. Patch forthcoming.

Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Priority: -- → P1

This is just so that both the launcher process and other Gecko code can share
this method.

For the URI handling case, we still want to parse the URI to look for any
malformation. OTOH, IShellDispatch2::ShellExecute does not accept PIDLs as
arguments, we we need an overload that converts the absolute PIDL back to a
path for the purposes of passing on to that interface.

Depends on D38943

Now that we have built up the required primatives in previous patches, this
patch simply replaces the previous code with a new version that uses the new
APIs from ShellHeaderOnlyUtils.h.

Depends on D38944

Flags: qe-verify+
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8bf96e8321a
Part 1 - Refactor launcher process's LaunchUnelevated to delegate to ShellExecuteByExplorer; r=jmathies
https://hg.mozilla.org/integration/autoland/rev/cae98337ddda
Part 2 - Add ShellExecuteByExplorer overload to handle absolute PIDL lists; r=jmathies
https://hg.mozilla.org/integration/autoland/rev/e7d500f650cc
Part 3 - Use ShellExecuteByExplorer in nsMIMEInfoWin; r=jmathies

Is this fixed with new release? As even after upgrading to Firefox 68.0.1(20190717172542), we are getting the same error while opening Skype meeting URL. Tested on 2 of the affected systems.

Are there any additional steps required to perform?

This is fixed in the Nightly channel. We need to proceed through some QA testing to confirm that the issue is fixed (and, more importantly, has not caused any new issues) before we can uplift this to beta, release, and ESR.

Duplicate of this bug: 1566970

[Tracking Requested - why for this release]: Windows users cannot properly start external programs that are registered on their systems as handlers for custom URI schemes.

Please request Beta approval on this as soon as you get a chance. I'm assuming that you'd like this on the radar for possible dot release inclusion for 68, so the sooner we get it on Beta the better.

Flags: needinfo?(aklotz)

Comment on attachment 9079880 [details]
Bug 1567614: Part 1 - Refactor launcher process's LaunchUnelevated to delegate to ShellExecuteByExplorer; r=jmathies!

Beta/Release Uplift Approval Request

  • User impact if declined: Windows users starting applications with custom URIs registered with their system will not be able to correctly start the registered application.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See bug description
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch re-uses well-tested code that is already out on release. Furthermore, since the feature in question is completely broken, even if this change still does not work 100% correctly, the situation can't be any worse than it already is!
  • String changes made/needed: None
Flags: needinfo?(aklotz)
Attachment #9079880 - Flags: approval-mozilla-beta?
Attachment #9079881 - Flags: approval-mozilla-beta?
Attachment #9079882 - Flags: approval-mozilla-beta?

Comment on attachment 9079880 [details]
Bug 1567614: Part 1 - Refactor launcher process's LaunchUnelevated to delegate to ShellExecuteByExplorer; r=jmathies!

Fixes a regression in Fx68 causing Windows users to be unable to correct start applications registered as custom URI handlers. Approved for 69.0b9 so we can get some testing in this before possible dot release inclusion.

Attachment #9079880 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9079881 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9079882 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I successfully reproduced the issue on Firefox 68.0.1 under Windows 10 (x64) using the STR from Comment 0.

The issue is no longer reproducible on latest Nightly 70.0a1 (2019-07-29) and Firefox 69.0b9 under Windows 10 (x64).

Same here - we use Firefox 68.0.1esr x64 on some test machines and are affected in a local-links environment. With latest nightly, problem is not reproducible any more.

Will this fix get it into Firefox 68.1.0esr?

(In reply to Robert Geißler from comment #21)

Will this fix get it into Firefox 68.1.0esr?

We are currently testing the fix on beta, and if that looks good then we will uplift to ESR.

Comment on attachment 9079880 [details]
Bug 1567614: Part 1 - Refactor launcher process's LaunchUnelevated to delegate to ShellExecuteByExplorer; r=jmathies!

Beta/Release Uplift Approval Request

  • User impact if declined: Windows users starting applications with custom URIs registered with their system will not be able to correctly start the registered application.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1569756
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): * The feature is completely broken on release, so even if the fix doesn't work 100%, it is still an improvement over what was there.
  • We're reusing existing code that is well tested on release via the launcher process.
  • String changes made/needed: None

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Launching of external programs for custom URI schemes is currently broken. This affects Firefox ESR's ability to start various enterprise products, including Skype for Business, vSphere client, the newest version of the Zoom client.
  • User impact if declined: Windows users starting applications with custom URIs registered with their system will not be able to correctly start the registered application.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): * The feature is completely broken on release, so even if the fix doesn't work 100%, it is still an improvement over what was there.
  • We're reusing existing code that is well tested on release via the launcher process.
  • String or UUID changes made by this patch: None
Attachment #9079880 - Flags: approval-mozilla-release?
Attachment #9079880 - Flags: approval-mozilla-esr68?
Attachment #9079881 - Flags: approval-mozilla-release?
Attachment #9079882 - Flags: approval-mozilla-release?
Attachment #9079881 - Flags: approval-mozilla-esr68?
Attachment #9079882 - Flags: approval-mozilla-esr68?

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

Comment on attachment 9079880 [details]
Bug 1567614: Part 1 - Refactor launcher process's LaunchUnelevated to delegate to ShellExecuteByExplorer; r=jmathies!

regression fix for 68.0.2

Attachment #9079880 - Flags: approval-mozilla-release?
Attachment #9079880 - Flags: approval-mozilla-release+
Attachment #9079880 - Flags: approval-mozilla-esr68?
Attachment #9079880 - Flags: approval-mozilla-esr68+
Attachment #9079881 - Flags: approval-mozilla-release?
Attachment #9079881 - Flags: approval-mozilla-release+
Attachment #9079881 - Flags: approval-mozilla-esr68?
Attachment #9079881 - Flags: approval-mozilla-esr68+
Attachment #9079882 - Flags: approval-mozilla-release?
Attachment #9079882 - Flags: approval-mozilla-release+
Attachment #9079882 - Flags: approval-mozilla-esr68?
Attachment #9079882 - Flags: approval-mozilla-esr68+

Note: bug 1570845 does not block releasing this in 68.0.2 and 68ESR.

The issue is fixed on Firefox 68.0.2 (20190807142214 from Treeherder) and on 68.0.2esr (20190807144252 from Treeherder) under Windows 10 (x64) using Skype for Business from the Windows Store.

Regressions: 1572970
Regressions: 1573051

Hmm, looks like you've broken opening links from Thunderbird, bug 1573051 which is a blocker, and bug 1573051.

Since we're building TB 68.0 ESR from our own branch, I'll have to back it out there. Not being able to open links from e-mail really obliterates an e-mail client :-(

BTW, can you still launch 3rd party apps passing a link with a reference, the #thingy?

Status: RESOLVED → VERIFIED

The change made for this bugfix seems to have cause an issue with our application. We use IdentityServer for our login. We use a .net desktop application to login via the browser and with the latest Firefox update, the IdentityModel.Client assembly returns this error:
"Malformed callback URL."

It worked prior to 68.0.2 and currently works fine with Chrome, Edge, and IE11.
Here is a stacktrace from Visual Studio:
at IdentityModel.Client.AuthorizeResponse.ParseRaw()
at IdentityModel.Client.AuthorizeResponse..ctor(String raw)
at IdentityModel.OidcClient.OidcClient.<ProcessResponseAsync>d__13.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at IdentityModel.OidcClient.OidcClient.<LoginAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at Daikin.DaikinTools.Authentication.AuthenticationService.<Login>d__66.MoveNext()

(In reply to cstaff16 from comment #32)

The change made for this bugfix seems to have cause an issue with our application. We use IdentityServer for our login. We use a .net desktop application to login via the browser and with the latest Firefox update, the IdentityModel.Client assembly returns this error:
"Malformed callback URL."

It worked prior to 68.0.2 and currently works fine with Chrome, Edge, and IE11.
Here is a stacktrace from Visual Studio:
at IdentityModel.Client.AuthorizeResponse.ParseRaw()
at IdentityModel.Client.AuthorizeResponse..ctor(String raw)
at IdentityModel.OidcClient.OidcClient.<ProcessResponseAsync>d__13.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at IdentityModel.OidcClient.OidcClient.<LoginAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at Daikin.DaikinTools.Authentication.AuthenticationService.<Login>d__66.MoveNext()

FYI, I added a new bug to report this: Bug 1574209

(In reply to cstaff16 from comment #33)

FYI, I added a new bug to report this: Bug 1574209

Thank you.

Regressions: 1574209
See Also: → 1571618
Flags: in-qa-testsuite?(bogdan.maris)

Added a regression test case to our External Software Affecting Firefox test suite.

Flags: in-qa-testsuite?(bogdan.maris) → in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.