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)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: sbadau, Assigned: bbondy)
Details
Attachments
(1 file, 1 obsolete file)
4.97 KB,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 2•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
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
Assignee | ||
Comment 4•12 years ago
|
||
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.
Assignee | ||
Comment 5•12 years ago
|
||
I'll push it out to oak before landing.
Assignee: nobody → netzen
Status: NEW → ASSIGNED
Attachment #631043 -
Flags: review?(robert.bugzilla)
Comment 6•12 years ago
|
||
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), >+ ®DWORDValueSize); >+ 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+
Assignee | ||
Comment 7•12 years ago
|
||
Implemented nits, pushing to try and Oak for further testing before landing.
Attachment #631043 -
Attachment is obsolete: true
Attachment #633525 -
Flags: review+
Assignee | ||
Comment 8•12 years ago
|
||
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.
Assignee | ||
Comment 9•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/1b68705cce98
Target Milestone: --- → mozilla16
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1b68705cce98
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 11•12 years ago
|
||
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.
Description
•