Bug 1342742 (CVE-2017-7766)

Arbitrary code execution as SYSTEM using Updater to overwrite updater.ini

RESOLVED FIXED in Firefox -esr52

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sebbity, Assigned: rstrong)

Tracking

({csectype-priv-escalation, sec-high})

51 Branch
mozilla55
Unspecified
Windows
Points:
---
Bug Flags:
sec-bounty +
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox-esr45 wontfix, firefox52 wontfix, firefox-esr5254+ fixed, firefox53 wontfix, firefox54+ fixed, firefox55 fixed)

Details

(Whiteboard: [adv-main54+][adv-esr52.2+])

Attachments

(7 attachments, 11 obsolete attachments)

12.17 KB, application/x-zip-compressed
Details
31.13 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
52.30 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
31.13 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
66.70 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
38.04 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
59.25 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
Posted file 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
Flags: needinfo?(robert.strong.bugs)
Tracking for 54 for now though I'm not sure which branch we know is affected.
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Flags: needinfo?(robert.strong.bugs)
Flags: sec-bounty?
Robert, this bug has been reported almost 3 weeks ago and it's sec-high.
Can you move it up in your queue?
Flags: needinfo?(robert.strong.bugs)
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?
Flags: needinfo?(robert.strong.bugs) → needinfo?(dveditz)
I think so. Will check the others and see if I underrated them.
Flags: needinfo?(dveditz)
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.
Posted patch Patch in progress (obsolete) — Splinter Review
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!
Attachment #8853880 - Flags: feedback?(mhowell)
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.
Attachment #8853880 - Flags: feedback?(mhowell) → feedback+
(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;
>+    }
Flags: needinfo?(mhowell)
(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.
Flags: needinfo?(mhowell)
Posted patch patch in progress (obsolete) — Splinter Review
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!
Attachment #8853880 - Attachment is obsolete: true
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.
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.***
Posted patch patch rev1 (obsolete) — Splinter Review
Fixed a couple of things exposed by eslint

Pushed to try again
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b39a7e9212e9429a60d9e3249abf87ca7d28beb1
Attachment #8855107 - Attachment is obsolete: true
Seb, thanks for the bug report and for the precise details and instructions! It is really appreciated.
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.
Attachment #8855119 - Flags: review?(mhowell)
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.
(Reporter)

Comment 19

2 years ago
No worries Robert - happy to help! Glad to see this fixed :)
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.
Attachment #8855119 - Flags: review?(mhowell) → review+
(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.
Posted patch patch rev2 (obsolete) — Splinter Review
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.
Attachment #8855119 - Attachment is obsolete: true
Attachment #8855689 - Flags: review+
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.
Attachment #8855689 - Flags: sec-approval?
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.
Attachment #8855689 - Flags: sec-approval? → sec-approval+
Whiteboard: [checkin on May 2]
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.
Flags: needinfo?(abillings)
Yes, Rob, I think that would be ok and you more than have my trust on these things. :-)
Flags: needinfo?(abillings)
I'm separating the client code from the test code since I will be changing the test code before this bug can land.
Just removes gonk changes
Attachment #8857761 - Flags: review+
Posted patch test patch after bug 1354850 (obsolete) — Splinter Review
Hi Matt, I'd like you to take another look at the test code changed quite a bit after bug 1354850.
Attachment #8856071 - Attachment is obsolete: true
Attachment #8857762 - Flags: review?(mhowell)
Attachment #8857762 - Flags: review?(mhowell) → review+
Talked with abillings and I'll land this on June 28 so it can bake a few days before uplifing.
Whiteboard: [checkin on May 2] → [checkin on June 28]
I meant 4/28
Whiteboard: [checkin on June 28] → [checkin on 4/28]
Forgot to cleanup the Mac command line when elevated.
Attachment #8855689 - Attachment is obsolete: true
Attachment #8856070 - Attachment is obsolete: true
Attachment #8857761 - Attachment is obsolete: true
Attachment #8860766 - Flags: review?(mhowell)
Attachment #8860766 - Flags: review?(mhowell) → review+
I have patches for beta and esr that I will attach soon.
Attachment #8857762 - Attachment is obsolete: true
Attachment #8863068 - Flags: review+
Whiteboard: [checkin on 4/28]
(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
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Flags: sec-bounty? → sec-bounty+
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
Attachment #8863532 - Flags: review+
Attachment #8863532 - Flags: approval-mozilla-beta?
Forgot to mention that I have also built beta with this patch and all update tests pass.
This also needs the test changes from bug 1354850
Attachment #8863534 - Flags: review+
[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.
Attachment #8863537 - Flags: review+
Attachment #8863537 - Flags: approval-mozilla-esr52?
This also needs the test changes from bug 1354850
Attachment #8863539 - Flags: review+
A local privilege escalation like this is more relevant to enterprise users than typical home users, and thus appropriate for ESR.
Group: toolkit-core-security → core-security-release
Comment on attachment 8863532 [details] [diff] [review]
client patch for beta

Sec-high, Beta54+
Attachment #8863532 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8863537 [details] [diff] [review]
client patch for esr52

Sec-high, ESR52.2+
Attachment #8863537 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
(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
(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.
Flags: qe-verify-
Whiteboard: [adv-main54+][adv-esr52.2+]
Alias: CVE-2017-7766
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.