1342742 - (CVE-2017-7766) Arbitrary code execution as SYSTEM using Updater to overwrite updater.ini

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Seb Patane (he/him)]

Attachment: poc + source.zip

An unprivileged, standard, Windows user can execute arbitrary code as SYSTEM using the Firefox Updater (via the Maintenance Service).

This is possible by combining two bugs. The first is that the Updater can be tricked into executing an arbitrary PostUpdate file by the updater.ini file after a successful action is performed. The second allows overwrite of any file as SYSTEM with partially attacker controlled data.

We use the overwrite bug to write a new updater.ini, which triggers an install of Firefox into an attacker controlled directory which allows arbitrary code execution via the maintenanceservice_installer.exe binary.


# Reproduction

I've attached a poc + source, to run it you'll need the full update.mar for the current installed version of Firefox [0] (or higher), as well as a full firefox installer [1]. The poc can be run as follows:

firefox_poc.exe payload.exe firefox-51.0.1.complete.mar "Firefox Setup 51.0.1.exe" C:\Program Files (x86)\Mozilla Firefox

The payload.exe is the file to be executed as SYSTEM. The final argument needs to be a legitimate Firefox installation directory, and will default to C:\Program Files(x86)\Mozilla Firefox if not supplied. One of the bugs relies on a small race condition, which has always executed in less than a minute for me but let me know if you have any problems. It will continue trying until it succeeds or you use ctrl+c to kill it.

The included payload.exe just runs "whoami > /foo.txt", which should create a foo.txt file in your drive root with the contents "nt authority\system".


# ExeRelPath in updater.ini allows execution of arbitrary Mozilla binaries using absolute paths

 The ExeRelPath entry in the C:\Program Files (x86)\Mozilla Firefox\updater.ini file is executed by the updater after a successful action is made. There are allowances made to prevent directory traversal using '..', however if an absolute file path is provided instead of a filename then PathAppendSafe [2] (which uses PathAppendW) will return the absolute file path instead.

 Eg. PathAppendW("C:\Program Files (x86)\Mozilla Firefox", "C:\some\mozilla.exe")
 		=> "C:\some\mozilla.exe"

The updater will happily execute this file, as long as it is signed by Mozilla with a current certificate. We can use then use these binaries in ways never intended to gain arbitrary code execution.

The avenue used in this proof of concept is to use the Firefox setup.exe to install Firefox into an attacker controlled directory [3], which then executes whatever is in the maintenanceservice_installer.exe file on post-install (in fairness, this would have no practical chance of being intercepted otherwise). I imagine there are similar avenues in other Mozilla signed binaries, however I have not investigated them.


# Arbitrary file overwrite with partial content injection

Soon after the updater starts, updater.cpp [4] calls LogInit, which calls GetTempFileNameW with the attacker-supplied patch directory to get a temp filename for the log file. By providing a file which starts with \\.\, GetTempFileNameW will just copy the input into the output buffer instead of providing a valid temp directory.

Eg. GetTempFileNameW("\\.\c:\target\file.txt", L"log", 0, mTmpFilePath)
		=> mTmpFilePath = "\\.\c:\target\file.txt"

Since it is essentially a normal file path, the log file will happily truncate and write to this file.

We can insert up to 260 characters at some point in the file using the fact that the working directory argument gets logged in updater.cpp [5]. The resulting file ends up looking something like the following (ABC's are user supplied):

	Performing a replace request
	PATCH DIRECTORY \\.\C:\target\file.txt
	INSTALLATION DIRECTORY C:\Program Files (x86)\Mozilla Firefox
	WORKING DIRECTORY AAAAAAAAAAAAAAA
	BBBBBBBBBBBBBBBBBB
	CCCCCCCCCCCCCCCCCC

	The apply-to directory must be the same as or a child of the installation directory! Exiting.


In order to reach the line of code which logs the working directory argument, we need to first avoid a failure of WriteStatusFile [6], otherwise the updater will exit early and we will only truncate the file without our content written. The reason WriteStatusFile fails is that it tries to move a file to <patch-dir>/update.status, which in this case is \\.\c:\target\file.txt\update.status, which is not possible because file.txt is a file.

However, if we create a directory junction in an attacker controlled directory, eg. c:\exploit\junction\ pointing to c:\target\, then we can supply that to the updater instead. Now after the updater has opened a file descriptor to file.txt, we race it to switch the junction out with a directory named file.txt before it has attempted the status file move. If we win the race, the updater creates c:\exploit\junction\file.txt\update.status and execution continues.


# Arbitrary code execution

Because GetPrivateProfileStringW [7] isn't very picky about the formatting of the updater.ini, we can use the arbitrary file overwrite to replace the C:\Program Files (x86)\Mozilla Firefox\updater.ini with one which looks like this:

	Performing a replace request
	PATCH DIRECTORY \\.\C:\temp\ff-exploit\overwrite-target\updater.ini
	INSTALLATION DIRECTORY C:\Program Files (x86)\Mozilla Firefox
	WORKING DIRECTORY 

	[PostUpdateWin]
	ExeRelPath=C:\temp\ff-exploit\setup-dir\ff_setup.exe
	ExeArg=/INI=C:\temp\ff-exploit\setup-dir\ff_setup.ini



	The apply-to directory must be the same as or a child of the installation directory! Exiting.

The ff_setup.ini has the InstallDirectoryPath entry set to an attacker controlled directory, eg. c:\temp\ff-exploit\new-firefox\ and the MaintenanceService entry set to true. The attacker just needs to copy their payload.exe into c:\temp\ff-exploit-new-firefox\maintenanceservice_installer.exe and keep it locked for shared reads (so it can't be overwritten on install, but can be executed).

Then, we trigger some action which will succeed, like staging an upate.mar file
 	-> which will execute the PostUpdateWin command
 	-> which will install a new copy of firefox into the attacker controlled c:\temp\ff-exploit\new-firefox\ directory
 	-> which will then run the maintenanceservice_install.exe payload as SYSTEM

Let me know if you need any more information. I tested on Windows 8.1 x64, Firefox 51.0.1

[0] - http://archive.mozilla.org/pub/firefox/releases/51.0.1/update/win32/en-US/firefox-51.0.1.complete.mar
[1] - http://archive.mozilla.org/pub/firefox/releases/51.0.1/win32/en-US/Firefox%20Setup%2051.0.1.exe

[2] - https://dxr.mozilla.org/mozilla-central/rev/f36062d04d165f6f6e781cf0633ffcbbebe6c273/toolkit/mozapps/update/updater/updater.cpp#2029
[3] - https://wiki.mozilla.org/Installer:Command_Line_Arguments

[4] - https://dxr.mozilla.org/mozilla-central/rev/f36062d04d165f6f6e781cf0633ffcbbebe6c273/toolkit/mozapps/update/updater/updater.cpp#2937
[5] - https://dxr.mozilla.org/mozilla-central/rev/f36062d04d165f6f6e781cf0633ffcbbebe6c273/toolkit/mozapps/update/updater/updater.cpp#2958
[6] - https://dxr.mozilla.org/mozilla-central/rev/f36062d04d165f6f6e781cf0633ffcbbebe6c273/toolkit/mozapps/update/updater/updater.cpp#2939

[7] - https://dxr.mozilla.org/mozilla-central/rev/f36062d04d165f6f6e781cf0633ffcbbebe6c273/toolkit/mozapps/update/updater/updater.cpp#2005

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 54 for now though I'm not sure which branch we know is affected.

Comment 2

[Frederik Braun [:freddy]]

Robert, this bug has been reported almost 3 weeks ago and it's sec-high.
Can you move it up in your queue?

Comment 3

[Robert Strong (they/them - no direct email)]

Will do.

dveditz, I've noticed that you haven't been giving sec-high for local exploits as of late. Should this be sec-high?

Comment 4

[Daniel Veditz [:dveditz]]

I think so. Will check the others and see if I underrated them.

Comment 5

[Robert Strong (they/them - no direct email)]

Just an update for drivers, I'm close to being finished with this patch but I still need to finish up the tests. I hope to have this submitted for review by no later than EOD Monday.

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: Patch in progress

Matt, I still have a couple of more things I want to add to this patch but I wanted to get your input on the current set of changes while I am adding them. Thanks!

Comment 7

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8853880 [details] [diff] [review]
Patch in progress

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

Didn't do a full review yet, but I do agree with the approach of just locking down _all_ the paths like this.

I'm a little worried about IsValidFullPath rejecting UNC paths; running off of a network share seems like a case we want to support (without requiring a drive mapping).

Also, this might be one of the things you're planning to add, but it's probably also a good idea to check the return code from GetTempFileName. Looks like it would have been returning 0 and setting last-error to 267 (directory name is invalid) while running this exploit.

Comment 8

[Robert Strong (they/them - no direct email)]

(In reply to Matt Howell [:mhowell] from comment #7)
> Comment on attachment 8853880 [details] [diff] [review]
> Patch in progress
> 
> Review of attachment 8853880 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Didn't do a full review yet, but I do agree with the approach of just
> locking down _all_ the paths like this.
> 
> I'm a little worried about IsValidFullPath rejecting UNC paths; running off
> of a network share seems like a case we want to support (without requiring a
> drive mapping).
It allows UNC server shares but not other UNC paths. Are there other UNC paths you are concerned about?

>+  // Check if the path is a UNC server share.
>+  if (origFullPath[0] == NS_T('\\')) {
>+    if (!PathIsUNCServerShareW(origFullPath)) {
>+      return false;
>+    }

Comment 9

[Molly Howell (she/her) (no longer active)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #8)
> It allows UNC server shares but not other UNC paths. Are there other UNC
> paths you are concerned about?

No, you're right, I was misinterpreting that code somehow. Please disregard.

But that did lead me to discover another issue with using PathIsUNCServerShare this way; it only returns true if the path is just to a share, and false for a directory within a share:
PathIsUNCServerShareW(L"\\\\server\\share") == TRUE
PathIsUNCServerShareW(L"\\\\server\\share\\dir") == FALSE

So that's not all that useful in this context.

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: patch in progress

Thanks for checking that call and this latest patch should cover it. Still have a few more calls I need to verify but that was the main one. I added a test for the failure case. I'm not sure if it would be ok to add an in tree test using a valid UNC server share path since tests that access the network aren't allowed. Perhaps it would be ok if I used a server name that wouldn't be used since from what I recall the concern is that tests should pass without access to the network resource which this test would.

Added the GetTempFileName checks for the cases that are of concern. I still need to double check if there are other cases.

The main last thing I want to add is checking that the registry entry for the install directory exists from the maintenance service but there might be a few more as I examine the code.

Thanks for the feedback!

Comment 11

[Robert Strong (they/them - no direct email)]

Drivers, keeping the existing tests running while adding the new restrictions is taking a little longer than I had thought it would. Another day or so and it should be ready to land on nightly.

Comment 12

[Robert Strong (they/them - no direct email)]

Attachment: Patch in progress

Comment 13

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04910a382a4ff11e4991fac9aeba56cc0eaafd71

Comment 14

[Robert Strong (they/them - no direct email)]

Without the patch I was able to reproduce and I get the following in the maintenanceservice.log

updater.exe was compared successfully to the installation directory updater.exe.
The updater.exe application contains the Mozilla updater identity.
The file "C:\Program Files (x86)\Nightly\updater.exe" is signed and the signature was verified.
Passed in path: 'C:\Program Files (x86)\Nightly\updater.exe'; Using this path for updating: 'C:\Program Files (x86)\Mozilla Maintenance Service\update\updater.exe'.
updater.exe was compared successfully to the installation directory updater.exe.
The updater.exe application contains the Mozilla updater identity.
The file "C:\Program Files (x86)\Mozilla Maintenance Service\update\updater.exe" is signed and the signature was verified.
Starting update process as the service in session 0.
Starting service with cmdline: "C:\Program Files (x86)\Mozilla Maintenance Service\update\updater.exe" C:\moz\170626\patch-dir "C:\Program Files (x86)\Nightly" "C:\Program Files (x86)\Nightly\updated" -1 . C:\moz\170626\setup-dir\ff_setup.ini
Process was started... waiting on result.
Process finished with return code 0.
updater.exe returned status: applied

updater.exe was launched and run successfully!
Service command software-update complete.
service command aaa complete with result: Success.

With the patch I get the following in the maintenanceservice.log

*** Warning: The patch directory path is not valid for this application.***

Comment 15

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Fixed a couple of things exposed by eslint

Pushed to try again
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b39a7e9212e9429a60d9e3249abf87ca7d28beb1

Comment 16

[Robert Strong (they/them - no direct email)]

Seb, thanks for the bug report and for the precise details and instructions! It is really appreciated.

Comment 17

[Robert Strong (they/them - no direct email)]

Comment on attachment 8855119 [details] [diff] [review]
patch rev1

I've run other versions of this patch through the try server so I am fairly certain it will pass so requesting review.

Comment 18

[Robert Strong (they/them - no direct email)]

I filed bug 1353889 which should make it possible to add tests for the one change that I didn't add a test for. I manually verified that change worked as expected. There are some other things that can be done to simplify the tests overall after the changes to TestAUSHelper but I don't want to do that in this bug.

Comment 19

[Seb Patane (he/him)]

No worries Robert - happy to help! Glad to see this fixed :)

Comment 20

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8855119 [details] [diff] [review]
patch rev1

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

r+ overall with a handful of comments. Thanks for getting this mess sorted out.

::: toolkit/mozapps/update/common/updatecommon.cpp
@@ +192,5 @@
> +  }
> +
> +  if (origFullPath[0] == NS_T('\\')) {
> +    // Only allow UNC server share paths.
> +    if (!PathIsUNCServerShareW(testPath)) {

Still need to fix this call; PathIsUNCServerShare rejects anything not of the precise form "\\server\share" (including "\\server\share\" it turns out), but we need to allow subdirectories of a share.

::: toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
@@ +2644,5 @@
> +
> +  // Make sure the service from the previous test is already stopped.
> +  waitForServiceStop(true);
> +
> +  // Prevent the cleanup function from begin run more than once

Typo for "being".

::: toolkit/mozapps/update/tests/unit_base_updater/invalidArgPatchDirPathTraversalFailure.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/.
> + */
> +
> +/* Install directory path traversal failure test */

This one is patch directory, not install directory.

::: toolkit/mozapps/update/tests/unit_base_updater/invalidArgWorkingDirPathLocalUNCFailure_win.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/.
> + */
> +
> +/* Too long install directory path failure test */

Working directory, not install directory, and UNC path, not overlong path.

::: toolkit/mozapps/update/tests/unit_service_updater/invalidArgWorkingDirPathLocalUNCFailureSvc_win.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/.
> + */
> +
> +/* Too long install directory path failure test */

Working directory, not install directory, and UNC path, not overlong path.

::: toolkit/mozapps/update/updater/updater.cpp
@@ +2151,5 @@
>    NS_tchar filename[MAXPATHLEN] = {NS_T('\0')};
>  #if defined(XP_WIN)
>    // The temp file is not removed on failure since there is client code that
>    // will remove it.
> +  if (GetTempFileNameW(gPatchDirPath, L"sta", 0, filename) == 0) {

This may not quite work; GetTempFileName returns 0 and sets the last error to 267 ("The directory name is invalid") when the first argument is a path that doesn't exist, even if it's a valid path. Might have to move the ensure_parent_dir call up here, or just check for that case.

Comment 21

[Molly Howell (she/her) (no longer active)]

(In reply to Matt Howell [:mhowell] from comment #20)
> ::: toolkit/mozapps/update/common/updatecommon.cpp
> @@ +192,5 @@
> > +  }
> > +
> > +  if (origFullPath[0] == NS_T('\\')) {
> > +    // Only allow UNC server share paths.
> > +    if (!PathIsUNCServerShareW(testPath)) {
> 
> Still need to fix this call; PathIsUNCServerShare rejects anything not of
> the precise form "\\server\share" (including "\\server\share\" it turns
> out), but we need to allow subdirectories of a share.

Disregard this comment; I didn't notice we were calling PathIsUNCServerShare on the result of PathStripToRoot.

Comment 22

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

Addressed comments except for the one noted in comment #21 and
> ::: toolkit/mozapps/update/updater/updater.cpp
> @@ +2151,5 @@
> >    NS_tchar filename[MAXPATHLEN] = {NS_T('\0')};
> >  #if defined(XP_WIN)
> >    // The temp file is not removed on failure since there is client code that
> >    // will remove it.
> > +  if (GetTempFileNameW(gPatchDirPath, L"sta", 0, filename) == 0) {
> 
> This may not quite work; GetTempFileName returns 0 and sets the last error
> to 267 ("The directory name is invalid") when the first argument is a path
> that doesn't exist, even if it's a valid path. Might have to move the
> ensure_parent_dir call up here, or just check for that case.

This isn't needed since the file is written to the patch directory which must exist.

Comment 23

[Robert Strong (they/them - no direct email)]

Comment on attachment 8855689 [details] [diff] [review]
patch rev2

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The steps to reproduce this are fairly complicated (see comment #0) so in my opinion it wouldn't be that easy,

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
I don't think it paints a bulls-eye on the security problem though the checks do give a starting point on what to go after.

Which older supported branches are affected by this flaw?
All

If not all supported branches, which bug introduced the flaw?
N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
It should be fairly simple and no more risky than this patch.

How likely is this patch to cause regressions; how much testing does it need?
As with all significant app update patches I definitely want this to bake on Nightly for a couple of days. Also, if it is ok with you I'd like to land this on the oak branch and test it before landing it on Nightly.

Comment 24

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8855689 [details] [diff] [review]
patch rev2

We're about to make release builds for 53. There is no way this can go in yet. I'm giving it sec-approval+ to land on *May 2*, two weeks into the next cycle, and not before then.

Comment 25

[Robert Strong (they/them - no direct email)]

Al, are you ok with my landing this on oak so I can test it there first and if so do you have a date constraint for my landing it. I won't reference this bug or give any details in the message used to land it there?

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #23)
<snip>
> As with all significant app update patches I definitely want this to bake on
> Nightly for a couple of days. Also, if it is ok with you I'd like to land
> this on the oak branch and test it before landing it on Nightly.

Comment 26

[Al Billings [:abillings - ex-MoCo]]

Yes, Rob, I think that would be ok and you more than have my trust on these things. :-)

Comment 27

[Robert Strong (they/them - no direct email)]

Attachment: patch for checkin - client code

I'm separating the client code from the test code since I will be changing the test code before this bug can land.

Comment 28

[Robert Strong (they/them - no direct email)]

Attachment: patch for checkin - test code

Comment 29

[Robert Strong (they/them - no direct email)]

Attachment: patch for m-c after bug 1156046

Just removes gonk changes

Comment 30

[Robert Strong (they/them - no direct email)]

Attachment: test patch after bug 1354850

Hi Matt, I'd like you to take another look at the test code changed quite a bit after bug 1354850.

Comment 31

[Robert Strong (they/them - no direct email)]

New try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b328911803230005f7f1fe1d3cb879b05f06c355

Comment 32

[Robert Strong (they/them - no direct email)]

Previous push had errors unrelated to this bug... new try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1c866057101c1b9e7c0ce62fe526fcf54acbd1d

Comment 33

[Robert Strong (they/them - no direct email)]

Per comment #24 this won't make it into 53

Comment 34

[Robert Strong (they/them - no direct email)]

Talked with abillings and I'll land this on June 28 so it can bake a few days before uplifing.

Comment 35

[Robert Strong (they/them - no direct email)]

I meant 4/28

Comment 36

[Robert Strong (they/them - no direct email)]

Attachment: patch for m-c checkin - client code

Forgot to cleanup the Mac command line when elevated.

Comment 37

[Robert Strong (they/them - no direct email)]

Attachment: pushed client patch for m-c

https://hg.mozilla.org/integration/mozilla-inbound/rev/702bca2e601ff093c89c4e3d9deac35bf28d2550

Comment 38

[Robert Strong (they/them - no direct email)]

Attachment: pushed test patch for m-c

I have patches for beta and esr that I will attach soon.

Comment 39

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #38)
> Created attachment 8863068 [details] [diff] [review]
> pushed test patch for m-c
> 
> I have patches for beta and esr that I will attach soon.
Note: pushed both patches to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b52d7c9212cfbf5266b0d1f0b8d35ba694a06a4

Comment 40

[Robert Strong (they/them - no direct email)]

Merged to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/702bca2e601f
https://hg.mozilla.org/mozilla-central/rev/5b52d7c9212c

I have patches for beta and esr that I will attach soon.

Comment 41

[Robert Strong (they/them - no direct email)]

Attachment: client patch for beta

Approval Request Comment
[Feature/Bug causing the regression]:
Original maintenance service implementation
[User impact if declined]:
They will continue to be vulnerable to this local exploit
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]:
I have verified it on Nightly
[Needs manual test from QE? If yes, steps to reproduce]:
No
[List of other uplifts needed for the feature/fix]:
N/A
[Is the change risky?]:
No more so than any other similar change to app update.
[Why is the change risky/not risky?]:
It validates the command line arguments used with the updater are what should be sent by Firefox. It is possible though not likely that some system might somehow munge the arguments. 
[String changes made/needed]:
None

Comment 42

[Robert Strong (they/them - no direct email)]

Forgot to mention that I have also built beta with this patch and all update tests pass.

Comment 43

[Robert Strong (they/them - no direct email)]

Attachment: test patch for beta

This also needs the test changes from bug 1354850

Comment 44

[Robert Strong (they/them - no direct email)]

Attachment: client patch for esr52

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
They will continue to be vulnerable to this local exploit
Fix Landed on Version:
55
Risk to taking this patch (and alternatives if risky): 
No more so than any other similar change to app update. It validates the command line arguments used with the updater are what should be sent by Firefox. It is possible though not likely that some system might somehow munge the arguments. 
String or UUID changes made by this patch: 
None

I have also built esr52 with this patch and all update tests pass.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 45

[Robert Strong (they/them - no direct email)]

Attachment: test patch for esr52

This also needs the test changes from bug 1354850

Comment 46

[Daniel Veditz [:dveditz]]

A local privilege escalation like this is more relevant to enterprise users than typical home users, and thus appropriate for ESR.

Comment 47

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8863532 [details] [diff] [review]
client patch for beta

Sec-high, Beta54+

Comment 48

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8863537 [details] [diff] [review]
client patch for esr52

Sec-high, ESR52.2+

Comment 49

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-beta
https://hg.mozilla.org/releases/mozilla-beta/rev/f7d1810de3816ea87d58c3828640f9939862ee2e

Comment 50

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #49)
> Pushed to mozilla-beta
> https://hg.mozilla.org/releases/mozilla-beta/rev/
> f7d1810de3816ea87d58c3828640f9939862ee2e
Also, the test patch
https://hg.mozilla.org/releases/mozilla-beta/rev/bbcddab3288afe64402e5093cf7786fe152bb4e7

Comment 51

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-esr52
https://hg.mozilla.org/releases/mozilla-esr52/rev/61066f53c6e0234c2f55ae43329d4b8d2b7b3b57
https://hg.mozilla.org/releases/mozilla-esr52/rev/c15b2a5abf1ca5c4169ef6340be56a25b5ec4f45

Comment 52

[Andrei Vaida [:avaida]]

(In reply to Robert Strong PTO 5/5 [:rstrong] (use needinfo to contact me) from comment #41)
> [Is this code covered by automated tests?]: Yes
> [Has the fix been verified in Nightly?]:
> I have verified it on Nightly
> [Needs manual test from QE? If yes, steps to reproduce]:
> No

Setting qe-verify- based on Robert's assessment on manual testing needs and the fact that this fix has automated coverage.