Closed Bug 1584907 (CVE-2019-15903) Opened 5 years ago Closed 5 years ago

Take patches from expat 2.2.8

Categories

(Core :: XML, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 70+ verified
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 blocking verified
firefox71 + verified

People

(Reporter: freddy, Assigned: peterv)

Details

(Keywords: csectype-bounds, sec-high, Whiteboard: [adv-main70+][adv-esr68.2+])

Attachments

(3 files)

security@mozilla.com received an email on September 14th. Surprisingly, there seems to be no Bugzilla entry for this issue.

If we want to fix just the heap overflow, then we need to look at the patch at:
https://github.com/libexpat/libexpat/commit/c20b758c332d9a13afbbb276d30db1d183a85d43

Am 14.09.19 um 23:00 schrieb Sebastian Pipping:

Hi!

I would like to let you know that Expat 2.2.8 [1] has been released. It
fixes heap buffer over-read CVE-2019-15903 [2] and other issues [3].

If you happen to have patches for Expat that are still required with
2.2.8, please send them my way.

Thank you!

Best

Sebastian

[1] https://github.com/libexpat/libexpat/releases/tag/R_2_2_8
[2] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-15903
[3] https://github.com/libexpat/libexpat/blob/R_2_2_8/expat/Changes

This heap overflow is already public, so my understanding is that this is urgent, but I'm happy to be convinced otherwise.
It seems that Peter is out, so I'm CCing bz.

Test case at https://github.com/libexpat/libexpat/issues/317
The test case requires code to call XML_GetCurrentLineNumber on the bad XML tree, but we seem to be doing that for errorneous documents.

Patch and discussion at https://github.com/libexpat/libexpat/pull/318

Flags: needinfo?(peterv)

I'll take a look to see if we can cherrypick.

Attachment #9097530 - Attachment description: Bug 1584907 - Deny internal entities closing the doctype. → Bug 1584907 - Deny internal entities closing the doctype. r?bzbarsky!

Verified with an asan build that this fixes it.

I'll open a a separate bug for getting us completely on 2.2.9.

Flags: needinfo?(peterv)
Summary: Update or take patches from expat 2.2.8 → Take patches from expat 2.2.8
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Priority: -- → P1

so I'm CCing bz

Fwiw, I was out yesterday as well...

Comment on attachment 9097530 [details]
Bug 1584907 - Deny internal entities closing the doctype. r?bzbarsky!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Triggering the heap overflow is trivial, the Expat source has a couple of examples in tests. They have been public since Expat 2.2.8 was released.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: All
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: I think the patch should apply as is to older branches.
  • How likely is this patch to cause regressions; how much testing does it need?: It doesn't look like this would cause a lot of regressions. It just catches an invalid construct in an XML document and throws an error, as opposed to continuing processing it.
Attachment #9097530 - Flags: sec-approval?

Noteworthy for responding to the approval process:

All of the information is already public. We might want to get this on more branches than just nightly - maybe even ride along with an upcoming dot-release.

I doubt there will be able more dot-releases for 69: we'll be building Firefox 70 release candidates in about a week or so. Given the upstream bug is public we should make sure this ships with all the branch releases.

Group: core-security → dom-core-security

Comment on attachment 9097530 [details]
Bug 1584907 - Deny internal entities closing the doctype. r?bzbarsky!

Missed this step last night: sec-approval+

Attachment #9097530 - Flags: sec-approval? → sec-approval+
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Please nominate this for Beta and ESR68 approval when you get a chance.

Flags: needinfo?(peterv)
Severity: normal → major

non-ASAN crash from the upstream public testcase which is consistent with the analysis in https://github.com/libexpat/libexpat/issues/317#issuecomment-525519310 -- bp-8dd5bcf8-48e9-469c-8fa1-7ac600191008

To get into the beta 14 build (tomorrow morning) we need to uplift the patches today.

Comment on attachment 9097530 [details]
Bug 1584907 - Deny internal entities closing the doctype. r?bzbarsky!

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: public patch for a sec-high bug
  • User impact if declined:
  • Fix Landed on Version: Fx 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Patch has been tested in the upstream expat library
  • String or UUID changes made by this patch: none

Beta/Release Uplift Approval Request

  • User impact if declined: Affected by public bug
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: open the attached testcase. Tab will usually crash if that version is affected. Verifying on beta/ESR is important.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Patch has been tested in the upstream expat library
  • String changes made/needed: none
Attachment #9097530 - Flags: approval-mozilla-esr68?
Attachment #9097530 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9097530 [details]
Bug 1584907 - Deny internal entities closing the doctype. r?bzbarsky!

Fixes a public expat sec issue. Approved for 70.0b14 and 68.2esr.

Flags: needinfo?(peterv)
Attachment #9097530 - Flags: approval-mozilla-esr68?
Attachment #9097530 - Flags: approval-mozilla-esr68+
Attachment #9097530 - Flags: approval-mozilla-beta?
Attachment #9097530 - Flags: approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
QA Whiteboard: [qa-triaged] → [qa-triaged][adv-main70+]
QA Whiteboard: [qa-triaged][adv-main70+] → [qa-triaged]
Whiteboard: [adv-main70+]
Whiteboard: [adv-main70+] → [adv-main70+][adv-esr68.2+]

Hi, This issue is Verified as fixed in our latest Nightly build, Beta 70.0b14 and 68.2esr on Windows 10, Ubuntu 18.04 and Mac OsX 10.14

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: