Bug 1237219 (CVE-2016-2826)

Mozilla maintenance service arbitrary file overwrite allowing privilege escalation

RESOLVED FIXED in Firefox 47

Status

()

Toolkit
Application Update
RESOLVED FIXED
2 years ago
3 months ago

People

(Reporter: Frédéric Hoguin, Assigned: mhowell)

Tracking

({sec-high})

43 Branch
mozilla49
sec-high
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox46 wontfix, firefox47+ fixed, firefox48+ fixed, firefox49+ fixed, firefox-esr38 wontfix, firefox-esr4547+ fixed)

Details

(Whiteboard: [adv-main47+][adv-esr45.2+])

Attachments

(2 attachments, 7 obsolete attachments)

(Reporter)

Description

2 years ago
Created attachment 8704556 [details]
Exploit source code

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151223140742

Steps to reproduce:

A combination of vulnerabilities in updater.cpp allows to overwrite arbitrary files, including protected files in protected folders (such as c:\windows), allowing privilege escalation (for example, by overwriting one of the many executables that are scheduled to run periodically with SYSTEM privileges).

The main vulnerability is that files extracted by ArchiveReader are not locked for writing and can be overwritten while the update is running.

Since a software-update command can be run in any location by unprivileged users, an attacker can choose to run it from a controlled location and overwrite files extracted by updater.exe. By overwriting files as they are being extracted, the attacker can force updater.exe to overwrite any file. I explain the procedure below (after "Steps to reproduce").

The updater will only accept to overwrite files that are in the update location and its subfolders (the updater rejects any path that is not relative, or that contains ".."). But it doesn't check if the path contains junctions, allowing to circumvent this protection. Unlike symlinks, junctions can be created by unprivileged users.

Using a combination of both these vulnerabilities allow to overwrite any file in any protected location.

This is all theoretical, so I wrote an exploit in order to prove the seriousness of this vulnerability. This has been tested successfully on Windows 7 and Windows 10 (both 64 bits) with Firefox 43.0.2 and 43.0.3 (32 bits).

Steps to reproduce:
* Download the FirefoxExploit.zip project and unzip it.
* If necessary, modify any of the configuration parameters in main.cpp (junctionName, fileToOverwrite, and defaultMarPatchFileToOverwrite). In the next steps I'll assume you're using the default values.
* Compile the project (I used VS2013, any recent version should be alright). You get FirefoxExploit.exe.
* From a privileged account, create a file in c:\windows named exploitpoc.txt and put some text in it (e.g. "Original content"). This is the file that will be overwritten as a proof of concept.
* Download the latest mar file (firefox-43.0.2-43.0.3.partial.mar: http://releases.mozilla.org/pub/firefox/releases/43.0.3/update/win32/en-US/firefox-43.0.2-43.0.3.partial.mar , replace en-US by the correct language).
* Optionally, create a limited user account on the computer you are going to run the exploit on (this is to prove the privilege escalation) and log in with it.
* Place FirefoxExploit.exe and the mar file in a directory. I'll assume it's c:\firefox_pe in the next steps.
* Create a file named exploitpocnew.txt in c:\firefox_pe. Put any content in it (e.g. "Exploit successful"); it will replace c:\windows\exploitpoc.txt after the exploit is successful.
* Open a cmd shell. Go to c:\firefox_pe.
* Create a junction named "exploit" which points to c:\windows : mklink /J exploit c:\windows
* Execute the exploit: FirefoxExploit.exe exploitpocnew.txt
* Open c:\windows\exploitpoc.txt . It now contains "Exploit successful".

Below is an explanation of how the exploit works. For complete details, refer to the source code (mainly main.cpp).

The exploit will first prepare some things:
* Detect the Firefox installation directory, copy updater.exe from it into c:\firefox_pe (so that the maintenance service accepts to run the update from this location).
* Open the mar file, make sure the version is correct (downgrade is refused), make sure it contains the files needed.
* Read the updatev3.manifest and the original patch from the mar file, so that it can detect later when updater.exe has decompressed enough data from them.
* Generate (using bsdiff) the rogue patch file that will overwrite the legitimate patch.

Then, it will start the update by launching the maintenance service with the appropriate parameters. As soon as it is launched, it will continually monitor the c:\firefox_pe\updating directory, where updater.exe puts files extracted from the mar archive.

The first step of the exploit is to continually check for the presence of update.manifest (extracted by updater.exe). As soon as it is here, it is continually read until enough data is written to it. It is then overwritten with our rogue manifest, which contains a single PatchFile action with the file we want to overwrite as a destination.

The second step is to continually check for the presence of 0.patch (this is updater.exe extracting the legitimate patch file from the mar). Again, as soon as enough data is written to it, it is overwritten with our rogue patch. At this point, it is still being written to by updater.exe, so it is repeatedly truncated to the size of our rogue patch.

Then, updater.exe will apply our rogue patch to the file we want to overwrite. The exploit is successful; you can check that c:\windows\exploitpoc.txt contains the content of exploitpocnew.txt ("Exploit successful").

I'll be happy to provide any help needed to run the exploit successfully. I strongly advise to run the exploit on a multi-core machine; the race condition is much more likely to succeed if it can run concurrently to updater.exe since it has busy loops.
Flags: sec-bounty?
Notes:
The last time we had a similar bug it was decided it would be too difficult to determine consistently if we are dealing with junction points.

Ideally the maintenance service will never be used with locations where non-admin users have write access.

If there is a decent way to consistently determine either of these two conditions (there might be other conditions we could check) then that would be a good approach to fixing this bug.
(Assignee)

Comment 2

2 years ago
There might be a different direction we could address this one from. This exploit is all about overwriting files while the update is in the process of being extracted from the MAR; if we could prevent those overwrite operations from succeeding, maybe by manipulating the permissions on the temporary files or by holding handles to them until we're done with them, then the actual location where all this is happening wouldn't matter, right?
That is definitely another approach. Since this has been a pita if multiple mitigations are possible I would be in favor of that as well.
Keywords: sec-high
Flags: sec-bounty? → sec-bounty+
Matt is this something you might want to take on? We came across it in sec triage today and wonder if it is actionable.
Flags: needinfo?(mhowell)
(Assignee)

Comment 5

a year ago
Sure, I can at least poke around and try to figure out how hard this is going to be. There might be some pretty simple mitigations.
Assignee: nobody → mhowell
Flags: needinfo?(mhowell)
(Assignee)

Comment 6

a year ago
Created attachment 8736489 [details] [diff] [review]
Patch

This patch works by making the directory the MAR is unpacked into inaccessible to non-admin users when the maintenance service is being used, so that the exploit code is unable to perform its "in-flight" edits to files in there. This seems to render the exploit ineffective (it hangs forever on my machines).

I've left the more general issue of what to do about junctions/links for another day (again). When I looked into that angle, it wasn't clear how to differentiate the malicious kind of junction point from the useful kind. I also couldn't find a way to tell junctions apart from volume mount points from within the Windows API, and I'm not in a hurry to risk problems for configurations using those.
Attachment #8736489 - Flags: review?(robert.strong.bugs)
(Assignee)

Comment 7

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=59123d5f15c0
(Assignee)

Comment 8

a year ago
Comment on attachment 8736489 [details] [diff] [review]
Patch

This patch blew up spectacularly on try (even though it passes locally, yay), so removing the review request until I can fix it.
Attachment #8736489 - Flags: review?(robert.strong.bugs)
I have to say that setting restrictive permissions to try to fix these types of issues always scares me since it could possibly lock out part of the process.
(Assignee)

Comment 10

a year ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #9)
> I have to say that setting restrictive permissions to try to fix these types
> of issues always scares me since it could possibly lock out part of the
> process.

That's definitely a concern. I thought that limiting this to the service would prevent locking anything out, because the rest of the process would also be under the service and would have the same permissions.

The other ideas I had for solutions from a similar angle were to either apply the permissions on each individual file that we extract/patch instead of the whole directory, or to leave the permissions alone but hold open a non-sharing handle on each file until we're finished with it. I didn't attempt either of those because they both would require major surgery on the ArchiveReader and probably on libmar, but I still think they're both plausible.
(Assignee)

Comment 11

a year ago
Created attachment 8736764 [details] [diff] [review]
Patch 2

Now with 100% less broken string length checking! Or at least (n>0)%.

Try push incoming.
Attachment #8736489 - Attachment is obsolete: true
(Assignee)

Comment 12

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=618c45582268
(Assignee)

Comment 13

a year ago
Created attachment 8736961 [details] [diff] [review]
Patch attempt 3

Thought I might try actually creating the directory before working with it, we'll see how that goes. Also corrected the error logging.

Side note: I am considerably weirded out by the fact that the first two patches, which just did not work, could still consistently pass local test runs. I won't be too surprised if this one also fails on try.
Attachment #8736764 - Attachment is obsolete: true
(Assignee)

Comment 14

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=669b672dede7
I killed your pending Win7 and WinXP xpcshell tests on that latest try push, because the four runs on Win8 left us with four dead machines from having files they were unable to remove in subsequent test runs.
(Assignee)

Comment 16

a year ago
Created attachment 8738209 [details] [diff] [review]
Patch attempt 4

All right, I'm trying this one last time, adding a class with a destructor to make sure that the new directory the patch creates always gets deleted. I'll keep an eye on the try push and if it doesn't go well I'll probably abandon this strategy entirely; if it's that hard to get right, it's just not a good idea.
Attachment #8736961 - Attachment is obsolete: true
(Assignee)

Comment 17

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebc6eb9c78fd
(Assignee)

Comment 18

a year ago
Comment on attachment 8738209 [details] [diff] [review]
Patch attempt 4

Finally got a green try run for this one.
Attachment #8738209 - Flags: review?(robert.strong.bugs)
Comment on attachment 8738209 [details] [diff] [review]
Patch attempt 4

Though it will require surgery on the archive reader I think that is safer in the end.
Attachment #8738209 - Flags: review?(robert.strong.bugs) → review-
(Assignee)

Comment 20

a year ago
Created attachment 8740136 [details] [diff] [review]
New Patch

Completely different approach from the previous patch, and also different from the other ideas I had before. This patch works by calling LockFile on each bsdiff patch after the file is created but before any data is extracted into it from the MAR. This ensures that only the updater process is able to read or write the patch file until after the locking handle is closed.

For this fix to be effective, the file stream has to be kept open between the extraction (or prepare) and applying (or execute) phases, so that the lock is never released across both phases; most of the code changes in this patch are just implementing that change.
Attachment #8738209 - Attachment is obsolete: true
Attachment #8740136 - Flags: review?(robert.strong.bugs)
(Assignee)

Comment 21

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f43bc9d182af
Comment on attachment 8740136 [details] [diff] [review]
New Patch

Much better

>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
>...
>@@ -1470,46 +1471,48 @@ PatchFile::Prepare()
>   // extract the patch to a temporary file
>   mPatchIndex = sPatchIndex++;
> 
>   NS_tsnprintf(spath, sizeof(spath)/sizeof(spath[0]),
>                NS_T("%s/updating/%d.patch"), gWorkingDirPath, mPatchIndex);
> 
>   NS_tremove(spath);
> 
>-  FILE *fp = NS_tfopen(spath, NS_T("wb"));
>-  if (!fp)
>+  mPatchStream = NS_tfopen(spath, NS_T("wb+"));
>+  if (!mPatchStream)
>     return WRITE_ERROR;
nit: when changes are made to code please update it to comply to the style guideline.
if (!mPatchStream) {
  return WRITE_ERROR;
}

> #ifdef XP_WIN
>+  // Lock the patch file, so it can't be messed with between
>+  // when we're done creating it and when we go to apply it.
>+  LockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1);
I think this should fail when it is not possible to lock the file. The msdn page only states what is filesystems are supported on Win 8 / in 2012. It would be a good thing to check what is supported on Win XP.

>+
>   char sourcefile[MAXPATHLEN];
>   if (!WideCharToMultiByte(CP_UTF8, 0, mPatchFile, -1, sourcefile, MAXPATHLEN,
>                            nullptr, nullptr)) {
>     LOG(("error converting wchar to utf8: %d", GetLastError()));
>     return STRING_CONVERSION_ERROR;
>   }
> 
>-  int rv = gArchiveReader.ExtractFileToStream(sourcefile, fp);
>+  int rv = gArchiveReader.ExtractFileToStream(sourcefile, mPatchStream);
> #else
>-  int rv = gArchiveReader.ExtractFileToStream(mPatchFile, fp);
>+  int rv = gArchiveReader.ExtractFileToStream(mPatchFile, mPatchStream);
> #endif
>-  fclose(fp);
>+
>   return rv;
> }
> 
> int
> PatchFile::Execute()
> {
>   LOG(("EXECUTE PATCH " LOG_S, mFile));
> 
>-  AutoFile pfile(NS_tfopen(spath, NS_T("rb")));
>-  if (pfile == nullptr)
>-    return READ_ERROR;
>-
>-  int rv = MBS_ReadHeader(pfile, &header);
>+  fseek(mPatchStream, 0, SEEK_SET);
>+
>+  int rv = MBS_ReadHeader(mPatchStream, &header);
>   if (rv)
>     return rv;
nit: go ahead and update this to comply to the style guideline.
if (rv) {
  return rv;
}

UnlockFile should be called as well otherwise we have to rely on the system which may not have unlocked the file in time when launching Firefox after an update.

I'd like to take another look after either these comments are implemented or answers are given as to why they don't need to be implemented. Thanks!
Attachment #8740136 - Flags: review?(robert.strong.bugs) → review-
(Assignee)

Comment 23

a year ago
Created attachment 8740455 [details] [diff] [review]
Amended patch

I made all the changes, except for these two comments:

> The msdn page only states what is filesystems are supported on
> Win 8 / in 2012. It would be a good thing to check what is supported on Win XP.

I tried calling LockFile on XP SP3, and it worked as intended. I think the table that's on MSDN about Win8/2012 is trying to explain how new features introduced in those version are supported by previously existing API's.

> UnlockFile should be called as well otherwise we have to rely on the system
> which may not have unlocked the file in time when launching Firefox after an
> update.

The file gets unlocked when the handle is closed, which, since it's an AutoFile, should always happen either at the end of PatchFile::Execute or (in error cases) at the end of DoUpdate.
Attachment #8740136 - Attachment is obsolete: true
Attachment #8740455 - Flags: review?(robert.strong.bugs)
(In reply to Matt Howell [:mhowell] from comment #23)
> Created attachment 8740455 [details] [diff] [review]
> Amended patch
> 
> I made all the changes, except for these two comments:
> 
> > The msdn page only states what is filesystems are supported on
> > Win 8 / in 2012. It would be a good thing to check what is supported on Win XP.
> 
> I tried calling LockFile on XP SP3, and it worked as intended. I think the
> table that's on MSDN about Win8/2012 is trying to explain how new features
> introduced in those version are supported by previously existing API's.
I'm somewhat concerned that it might not be supported on all filesystems in earlier versions of windows which might be why they called this out for Win 8 here. I'll look into it.

> 
> > UnlockFile should be called as well otherwise we have to rely on the system
> > which may not have unlocked the file in time when launching Firefox after an
> > update.
> 
> The file gets unlocked when the handle is closed, which, since it's an
> AutoFile, should always happen either at the end of PatchFile::Execute or
> (in error cases) at the end of DoUpdate.
From
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365202%28v=vs.85%29.aspx

"If a process terminates with a portion of a file locked or closes a file that has outstanding locks, the locks are unlocked by the operating system. However, the time it takes for the operating system to unlock these locks depends upon available system resources. Therefore, it is recommended that your process explicitly unlock all files it has locked when it terminates. If this is not done, access to these files may be denied if the operating system has not yet unlocked them."

In my experience, these types of warnings are best heeded. Also, the page for LockFile doesn't state that the file is unlocked when the file handle is closed.
(Assignee)

Comment 25

a year ago
Created attachment 8740636 [details] [diff] [review]
Further amended patch

Added calls to UnlockFile.

I'm highly confident about LockFile being supported on Windows < 8. I tried it on XP and it worked, the SDK headers don't have it behind any version checks, and the specific features MSDN calls out for Win8/2012 support are all new features in Win8/2012, so it seems like the only purpose of that table is to specifically show support for the new features. I also found a Stack Overflow answer from the Vista cycle (http://stackoverflow.com/questions/853805/locking-files-using-c-on-windows) which mentions LockFile.
Attachment #8740455 - Attachment is obsolete: true
Attachment #8740455 - Flags: review?(robert.strong.bugs)
(Assignee)

Updated

a year ago
Attachment #8740636 - Flags: review?(robert.strong.bugs)
Comment on attachment 8740636 [details] [diff] [review]
Further amended patch

>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
>@@ -1372,22 +1372,33 @@ private:
>   static int sPatchIndex;
> 
>   const NS_tchar *mPatchFile;
>   const NS_tchar *mFile;
>   int mPatchIndex;
>   MBSPatchHeader header;
>   unsigned char *buf;
>   NS_tchar spath[MAXPATHLEN];
>+  AutoFile mPatchStream;
> };
> 
> int PatchFile::sPatchIndex = 0;
> 
> PatchFile::~PatchFile()
> {
>+  // Make sure mPatchStream gets unlocked on Windows; the system will do that,
>+  // but not until some indeterminate future time, and we want determinism.
>+  // Normally this happens at the end of Execute, when we close the stream;
>+  // this call is here in case Execute errors out.
>+#ifdef XP_WIN
>+  if (mPatchStream) {
>+    UnlockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1);
>+  }
>+#endif
>+
>   // delete the temporary patch file
>   if (spath[0])
>     NS_tremove(spath);
nit: please add braces while you are here

> 
>   if (buf)
>     free(buf);
nit: please add braces while you are here

> }
> 
>@@ -1470,48 +1481,55 @@ PatchFile::Prepare()
>   // extract the patch to a temporary file
>   mPatchIndex = sPatchIndex++;
> 
>   NS_tsnprintf(spath, sizeof(spath)/sizeof(spath[0]),
>                NS_T("%s/updating/%d.patch"), gWorkingDirPath, mPatchIndex);
> 
>   NS_tremove(spath);
> 
>-  FILE *fp = NS_tfopen(spath, NS_T("wb"));
>-  if (!fp)
>+  mPatchStream = NS_tfopen(spath, NS_T("wb+"));
>+  if (!mPatchStream) {
>     return WRITE_ERROR;
>+  }
> 
> #ifdef XP_WIN
>+  // Lock the patch file, so it can't be messed with between
>+  // when we're done creating it and when we go to apply it.
>+  if (!LockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1)) {
>+    LOG(("Couldn't lock patch file: %d", GetLastError()));
>+    return WRITE_ERROR;
What do you think about adding a new error code for this?

>+  }
Attachment #8740636 - Flags: review?(robert.strong.bugs)
(Assignee)

Comment 27

a year ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #26)
> > #ifdef XP_WIN
> >+  // Lock the patch file, so it can't be messed with between
> >+  // when we're done creating it and when we go to apply it.
> >+  if (!LockFile((HANDLE)_get_osfhandle(fileno(mPatchStream)), 0, 0, -1, -1)) {
> >+    LOG(("Couldn't lock patch file: %d", GetLastError()));
> >+    return WRITE_ERROR;
> What do you think about adding a new error code for this?

Seems like a good plan. How about LOCK_ERROR_PATCH_FILE, code 73?

I'll make the other changes as well.
(Assignee)

Comment 28

a year ago
Created attachment 8741422 [details] [diff] [review]
Patch

Now with new error code and a nice, healthy portion of braces.
Attachment #8740636 - Attachment is obsolete: true
Attachment #8741422 - Flags: review?(robert.strong.bugs)
Comment on attachment 8741422 [details] [diff] [review]
Patch

Note: I am fairly certain that the new error code does the "right thing" wen handling update errors here but we should know more from telemetry.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1186

Could a reworked version of this exploit be used for the AddFile case? Specifically the code in
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#1310

If so, please file a followup security bug for it.
Attachment #8741422 - Flags: review?(robert.strong.bugs) → review+
(Assignee)

Comment 30

a year ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #29)
> Could a reworked version of this exploit be used for the AddFile case?
> Specifically the code in
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/
> updater.cpp#1310
> 
> If so, please file a followup security bug for it.

You could get the file overwriting behavior if you beat the updater in a race, because fopen() on Windows enables sharing. But I don't think you get the privilege elevation (and therefore the security issue) because AddFile::Execute extracts the file in place, so you have to have permissions to write there anyway.
The patch files are extracted under <path to install dir>\updating so if it possible for patch files I think it should be possible for add files as well which backup the existing file and then extracts on top of the existing file.
(Assignee)

Comment 32

a year ago
Comment on attachment 8741422 [details] [diff] [review]
Patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily, I think. Any exploit would require already having local unprivileged code execution, and even then the procedure is not exactly simple or obvious (see bug description).

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
I do not believe so.

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?
I don't; they would be trivial to create and not at all risky, in my opinion.

How likely is this patch to cause regressions; how much testing does it need?
Regressions seem unlikely since the change is very simple and a green try run exists (this code already has test coverage).
Attachment #8741422 - Flags: sec-approval?
We're too late in the release cycle to take it this release. We'd want at least a couple of betas. I'm giving this sec-approval+ to check into trunk on May 10. Please check this in there then.

We'll want it nominated for Aurora, Beta, and ESR45 at that time as well so it can go in everywhere else following trunk.
status-firefox46: --- → wontfix
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox-esr38: --- → affected
status-firefox-esr45: --- → affected
Whiteboard: [checkin on 5/10]
Attachment #8741422 - Flags: sec-approval? → sec-approval+
Also, please let this bake on trunk for at least a week so we can check telemetry for any errors.
(Assignee)

Comment 35

a year ago
Per comment 33, this should have been checked in already. I don't have level 3, so I couldn't do it myself, and I forgot to request it until just now.
Keywords: checkin-needed
(In reply to Matt Howell [:mhowell] from comment #35)
> Per comment 33, this should have been checked in already. I don't have level
> 3, so I couldn't do it myself, and I forgot to request it until just now.

np, done!

https://hg.mozilla.org/integration/mozilla-inbound/rev/3fe6c728ce7674ff710309587eaae01ca46e887d
Keywords: checkin-needed
Whiteboard: [checkin on 5/10]
https://hg.mozilla.org/mozilla-central/rev/3fe6c728ce76
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
I imagine this code hasn't changed much-- does this patch apply cleanly to ESR-45 and Beta? If so request approval on this patch, if not we'll need munged versions first.
status-firefox-esr38: affected → wontfix
tracking-firefox47: --- → +
tracking-firefox48: --- → +
tracking-firefox49: --- → +
tracking-firefox-esr45: --- → 47+
Flags: needinfo?(mhowell)
(Assignee)

Comment 39

a year ago
(In reply to Daniel Veditz [:dveditz] from comment #38)
> I imagine this code hasn't changed much-- does this patch apply cleanly to
> ESR-45 and Beta? If so request approval on this patch, if not we'll need
> munged versions first.
The patch does apply cleanly to both ESR45 and beta. I'll make those requests.
Flags: needinfo?(mhowell)
(Assignee)

Comment 40

a year ago
Comment on attachment 8741422 [details] [diff] [review]
Patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
N/A; this is a sec-high bug.

User impact if declined: 
Exposure to sec-high vulnerability.

Fix Landed on Version:
Nightly 49. This request is for ESR45 and beta 47.

Risk to taking this patch (and alternatives if risky): 
Low; patch passed existing testing and has been running for a week on Nightly with no reported issues.

String or UUID changes made by this patch: 
None.
Attachment #8741422 - Flags: approval-mozilla-esr45?
Attachment #8741422 - Flags: approval-mozilla-beta?
(Assignee)

Comment 41

a year ago
Comment on attachment 8741422 [details] [diff] [review]
Patch

See comment 40
Attachment #8741422 - Flags: approval-mozilla-aurora?
Comment on attachment 8741422 [details] [diff] [review]
Patch

Sec-high, Aurora48+, Beta47+, ESR45+
Attachment #8741422 - Flags: approval-mozilla-esr45?
Attachment #8741422 - Flags: approval-mozilla-esr45+
Attachment #8741422 - Flags: approval-mozilla-beta?
Attachment #8741422 - Flags: approval-mozilla-beta+
Attachment #8741422 - Flags: approval-mozilla-aurora?
Attachment #8741422 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc2c791cbd98
https://hg.mozilla.org/releases/mozilla-beta/rev/fdb6793f1e4a
https://hg.mozilla.org/releases/mozilla-esr45/rev/25271e7d1121
status-firefox47: affected → fixed
status-firefox48: affected → fixed
status-firefox-esr45: affected → fixed
Group: toolkit-core-security → core-security-release
Whiteboard: [adv-main47+][adv-esr45.2+]
Alias: CVE-2016-2826
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.