Closed Bug 740842 Opened 12 years ago Closed 12 years ago

When updating Firefox the service is used although the UAC is off on Windows 8

Categories

(Toolkit :: Application Update, defect)

12 Branch
x86
Windows 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: sbadau, Assigned: bbondy)

Details

Attachments

(1 file, 1 obsolete file)

On Windows 8 Consumer Preview the service is used when updating even if the UAC is off.

Reproducible: always

Prerequisites: - disable the UAC

Steps to reproduce:
1. Install Firefox 
2. Go to Help menu -> About Firefox
3. Wait for the download to complete and click on the "Apply Updates" button.

Expectected results:
The update is done without using the service.

Actual results:
The service is used when updating (maintenanceservice.log is generated)
This happens because in Windows 8, UAC is never really disabled (unless you change some config in the registry).
Even if you have UAC "disabled", most processes are actually running in medium integrity level. If you launch a program that requires admin privileges, the system automatically opens it with high integrity, with no user interaction, but UAC is still there.
We don't do a check if UAC is disabled though, we do a write access check and determine if the service needs to be used. So this still needs to be looked into.
Summary: When updating Firefox the service is used although the UAC is off → When updating Firefox the service is used although the UAC is off on Windows 8
Confirmed Comment 1 from Andrea.
The updater on Windows 7 and below run as high integrity process to begin with when UAC is off.  

On Windows 8 since processes start as medium integrity by default when UAC is off, we see that we can't write into the installation directory and hence use the service.

We should instead on Windows 8 shell execute when UAC is off because you can elevate that way without a UAC prompt.
Attached patch Patch v1. (obsolete) — Splinter Review
I'll push it out to oak before landing.
Assignee: nobody → netzen
Status: NEW → ASSIGNED
Attachment #631043 - Flags: review?(robert.bugzilla)
Comment on attachment 631043 [details] [diff] [review]
Patch v1.

>diff --git a/toolkit/mozapps/update/common/updatehelper.cpp b/toolkit/mozapps/update/common/updatehelper.cpp
>--- a/toolkit/mozapps/update/common/updatehelper.cpp
>+++ b/toolkit/mozapps/update/common/updatehelper.cpp
>@@ -643,8 +643,60 @@ IsLocalFile(LPCWSTR file, BOOL &isLocal)
>     return FALSE;
>   }
> 
>   wcscpy(rootPath, file);
>   PathStripToRootW(rootPath);
>   isLocal = GetDriveTypeW(rootPath) == DRIVE_FIXED;
>   return TRUE;
> }
>+
>+
>+/**
>+ * Determines the DWORD value of a registry key value
>+ *
>+ * @param key       The base key to where the value name exists
>+ * @param valueName The name of the value
>+ * @param retValue  Out parameter which will hold the value
>+ * @param return    TRUE on success
@return instead of @param

>+*/
>+static BOOL
>+GetDWORDValue(HKEY key, LPCWSTR valueName, DWORD &retValue)
>+{
>+  DWORD regDWORDValueSize = sizeof(DWORD);
>+  LONG retCode = RegQueryValueExW(key, valueName, 0, NULL, 
>+                                  reinterpret_cast<LPBYTE>(&retValue),
>+                                  &regDWORDValueSize);
>+  return ERROR_SUCCESS == retCode;
>+}
>+
>+/**
>+ * Determines if the the system's elevation type allows
>+ * unprmopted elevation.
>+ *
>+ * @param isUnpromptedElevation Out parameter which specifies if unprompted
>+ *                              elevation is allowed.
>+ * @return TRUE if the value was obtained successfully.
>+*/
>+BOOL
>+IsUnpromptedElevation(BOOL &isUnpromptedElevation)
>+{
>+  LPCWSTR UACBaseRegKey =
>+    L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\System";
>+  HKEY baseKey;
>+  LONG retCode = RegOpenKeyExW(HKEY_LOCAL_MACHINE, 
>+                               UACBaseRegKey, 0,
>+                               KEY_READ, &baseKey);
>+  if (retCode != ERROR_SUCCESS) {
>+    return FALSE;
>+  } 
>+
>+  DWORD enabled, consent, secureDesktop;
>+  BOOL success = GetDWORDValue(baseKey, L"EnableLUA", enabled);
>+  success = success && 
>+            GetDWORDValue(baseKey, L"ConsentPromptBehaviorAdmin", consent);
>+  success = success &&
>+            GetDWORDValue(baseKey, L"PromptOnSecureDesktop", secureDesktop);
>+  isUnpromptedElevation = enabled && !consent && !secureDesktop;
>+
>+  RegCloseKey(baseKey);
>+  return success;
>+}
I am fairly certain that a reboot is necessary for this to take affect. Please add a comment to the affect that this value in the registry might not reflect reality due to this.

>diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp
>--- a/toolkit/mozapps/update/updater/updater.cpp
>+++ b/toolkit/mozapps/update/updater/updater.cpp
>@@ -2352,16 +2352,29 @@ int NS_main(int argc, NS_tchar **argv)
>       // Make sure the path to the updater to use for the update is on local.
>       // We do this check to make sure that file locking is available for
>       // race condition security checks.
>       if (useService) {
>         BOOL isLocal = FALSE;
>         useService = IsLocalFile(argv[0], isLocal) && isLocal;
>       }
> 
>+      // If we have unprompted elevation we might as well not use the
>+      // service for the update because we would be using more privs than
>+      // we need to update with.  Service updates use the SYSTEM account.
because we would be using the SYSTEM account which has more privs that we need to update with.

or similar.

>+      // Windows 8 has this user selectable as the new UAC off.  Other
>+      // Windows versions can have this if the user sets it explicitly in
>+      // their registry.
Windows 8 provides a user interface so users can configure this behavior and it can be configured in the registry in all Windows versions that support UAC.
Attachment #631043 - Flags: review?(robert.bugzilla) → review+
Attached patch Patch v2Splinter Review
Implemented nits, pushing to try and Oak for further testing before landing.
Attachment #631043 - Attachment is obsolete: true
Attachment #633525 - Flags: review+
This passes tests on Oak and upgrades fine <win8.  Also verified that it bypasses the use of the service when UAC is off on win8 which was the patches intent.
https://hg.mozilla.org/mozilla-central/rev/1b68705cce98
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Verified as fixed on Firefox 16 beta - updated Firefox 16 beta 4 to Firefox 16 beta 5 (having the UAC off) and the update is not done through the service anymore (maintenanceservice.log is not generated).

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: