Open Bug 1676840 Opened 4 years ago Updated 1 year ago

Can no longer bypass UAC prompt if Firefox is installed in non-default location

Categories

(Toolkit :: Application Update, defect)

defect

Tracking

()

People

(Reporter: emk, Unassigned)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [fidedi-ope])

Steps to reproduce:

  1. Install Firefox 81 in a non-default location. Make sure that the install directory is not user-writable.
  2. Run update and restart.

Actual result:
UAC prompt appears.

Expected result:
Maintenance service should launch the updater for me.

I have to enter a credential because I'm using a standard user. This is really annoying. Even worse, if I change the install location to C:\Proram Files, I can't inherit my user profile due to the "dedicated profile per installation" feature.

Apparently, this is an intentional change to fix CVE-2020-15663 (bug 1643199). But, could you please actually check whether the install location is user-writable instead of hard-coding allowed directories?

Regressed by: CVE-2020-15663
Has Regression Range: --- → yes

I can't add bug 1643199 to the "Regressed by:" field. (maybe because it is security confidential?) Needinfo-ing relevant developers instead.

Flags: needinfo?(ksteuber)
Flags: needinfo?(agashlin)
Flags: needinfo?(mhowell)

Apparently, this is an intentional change to fix CVE-2020-15663 (bug 1643199). But, could you please actually check whether the install location is user-writable instead of hard-coding allowed directories?

Or could you compare file versions between updater.exe and the target firefox.exe to prevent the downgrade attack?

I see the regression's already been added. I'll ask that in the future you please refrain from needinfo'ing most of the developers working in a component on a new bug filed in that component unless it is some kind of emergency; I assure you, we would have seen this without the flags.

If the install directory is writable by that non-admin user and also if the installer that created that copy was run without elevation, then you should be able to work around this by disabling the maintenance service; you can do that by clearing the checkbox called "Use a background service to install updates" in about:preferences. That should cause the update service to directly check whether it's able to write to that directory and continue updating if so. If the installer was originally run as an administrator, this may cause other problems because registry entries will not be writable by the non-admin user and so will not get updated unless elevation is performed, but with the service disabled and the write check passing I don't believe elevation would ever be attempted.

One thing we could do to help with this would be to check the install path in shouldUseService; currently we know that the service will refuse to run on a non-Program Files path, but the way that code path is organized we're still trying to invoke it anyway.

Flags: needinfo?(mhowell)
Flags: needinfo?(ksteuber)
Flags: needinfo?(agashlin)

I see the regression's already been added.

I still do not see the regressions. Maybe security-confidential regressions are visible only if the user has privilege to see the security-confidential bug itself?

I'll ask that in the future you please refrain from needinfo'ing most of the developers working in a component on a new bug filed in that component unless it is some kind of emergency; I assure you, we would have seen this without the flags.

Sorry, I'll be more prudent next time.

If the install directory is writable by that non-admin user and also if the installer that created that copy was run without elevation, then you should be able to work around this by disabling the maintenance service;

In this case, the updater did not ask elevation regardless of the maintenance service setting. Is it unexpected?
Anyway, the install directory is not writable in my case. (I'll update STR to clarify that.) So this workaround did not help.

The severity field is not set for this bug.
:nalexander, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(nalexander)

I think this is S4 because it's a very edgy edge case and because we explicitly want this behaviour.

I think there are two outstanding questions:

  1. Can we change the logic around the checking before requesting elevation? I.e., "could you please actually check whether the install location is user-writable instead of hard-coding allowed directories?" and, "could you compare file versions between updater.exe and the target firefox.exe to prevent the downgrade attack?".

Molly can have the last word here but the rule of thumb is that junctions and races mean that these types of dynamic properties of the system can be exploited. Experience is not in our favour here. We believe that the vast majority of non-writable installations are in the given hardcoded locations, so doing the simplest possible thing is reasonable.

  1. "In this case, the updater did not ask elevation regardless of the maintenance service setting. Is it unexpected?"

I'm having a hard time understanding what was tried and what happened. emk, can you detail exactly what steps you took and what you observed? There are a lot of things in play here: registry keys in HKLM, for example, can subtly impact the behaviour of the MMS, etc.

Severity: -- → S4
Flags: needinfo?(nalexander)
Flags: needinfo?(mhowell)
Flags: needinfo?(VYV03354)

(In reply to Nick Alexander :nalexander [he/him] from comment #6)

Molly can have the last word here but the rule of thumb is that junctions and races mean that these types of dynamic properties of the system can be exploited.

Exactly. We've had exploitable bugs in the past that came out of getting subtle things wrong here. It's also proven prohibitively difficult to actually answer the question of whether a location is meant to be user-writable; the only certain way is to actually attempt a write to a given path, but that action itself can open up attack surface, and I've tried before to answer that question by directly examining the ACLs and not had success because of the unpredictability and potential complexity of possible configurations. What we have done here represents an intentional tradeoff that improves security for the great majority of our Windows users.

Flags: needinfo?(mhowell)

(In reply to Nick Alexander :nalexander [he/him] from comment #6)

I'm having a hard time understanding what was tried and what happened. emk, can you detail exactly what steps you took and what you observed? There are a lot of things in play here: registry keys in HKLM, for example, can subtly impact the behaviour of the MMS, etc.

  1. Launch an old Nightly full installer (not a stub installer).
  2. Click [No] on the UAC dialog to decline elevation. But installation will continue.
  3. Install Nightly with the default setting. The install path will be user-writable.
  4. Make sure the maintenance service is enabled.
  5. Click [Help] > [About Nightly] to update.

Actual result: The update did not ask elevation.
Expected result (?): The update asks elevation.

Flags: needinfo?(VYV03354)

(In reply to Masatoshi Kimura [:emk] from comment #8)

(In reply to Nick Alexander :nalexander [he/him] from comment #6)

I'm having a hard time understanding what was tried and what happened. emk, can you detail exactly what steps you took and what you observed? There are a lot of things in play here: registry keys in HKLM, for example, can subtly impact the behaviour of the MMS, etc.

  1. Launch an old Nightly full installer (not a stub installer).
  2. Click [No] on the UAC dialog to decline elevation. But installation will continue.
  3. Install Nightly with the default setting. The install path will be user-writable.
  4. Make sure the maintenance service is enabled.
  5. Click [Help] > [About Nightly] to update.

Actual result: The update did not ask elevation.
Expected result (?): The update asks elevation.

The result you see is the result that I expect: this is an unprivileged installation that the user can update without elevation. The maintenance service is irrelevant here since it's not required.

Whiteboard: [fidedi-ope]
You need to log in before you can comment on or make changes to this bug.