Closed Bug 1754724 Opened 3 years ago Closed 3 years ago

Recent expat CVEs

Categories

(Core :: XML, defect)

Product:
Core
Component:
XML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
99 Branch
Tracking Flags:
Tracking Status
firefox-esr91 98+ fixed
firefox97 --- wontfix
firefox98 + fixed
firefox99 + fixed

People

(Reporter: jhorak, Assigned: peterv)

References

Details

(Keywords: sec-high, Whiteboard: sec-high for ESR which has no RLBox[sec-survey])

Attachments

(3 files, 1 obsolete file)

Bug 1754724 - Clear up some computations in expat code. r?farre!
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-release+
RyanVM
: approval-mozilla-esr91+
tjr
: sec-approval+
Details | Review
Bug 1754724 - Clear up some more computations in expat code. r?farre!
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-release+
RyanVM
: approval-mozilla-esr91+
tjr
: sec-approval+
Details | Review
Bug 1754724 - Clear up even more computations in expat code. r?farre!
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-release+
RyanVM
: approval-mozilla-esr91+
tjr
: sec-approval+
Details | Review

Lately some expat CVEs popped up [1], the expat is sandboxed in the version 96+
but the ESR seems not to be covered. Could you please investigate if the vulnerabilities has any relevancy for the Firefox?

[1] https://nvd.nist.gov/vuln/search/results?form_type=Basic&results_type=overview&query=expat&search_type=all&isCpeNameSearch=false

[Tracking Requested - why for this release]: possible sec issues

Group: core-security → dom-core-security
status-firefox-esr91: --- → affected
tracking-firefox-esr91: --- → ?
Severity: -- → S3
Keywords: sec-high
Whiteboard: sec-high for ESR which has no RLBox

Bobby, do you think RLBoxing expat on ESR was feasible?
(as a possible alternative to updating expat)

Flags: needinfo?(bholley)

  • CVE-2021-45960, CVE-2021-46143, CVE-2022-22822 to CVE-2022-22827: needs to be verified, but on first glance I don't think we allow enough data into the parser to hit these.

  • CVE-2022-23852: doesn't affect us, only affects "configurations with a nonzero XML_CONTEXT_BYTES", we don't set XML_CONTEXT_BYTES to nonzero.

  • CVE-2022-23990: on first glance I don't think we're affected: the patch has a comment saying "when there is an element declaration handler present (from a prior call to XML_SetElementDeclHandler)". We never set an element declaration handler.

  • CVE-2022-25236: on first glance I don't think we're affected: our namespace separator (0xFFFF) shouldn't make it into the data being parsed in the first place.

  • CVE-2022-25313 needs to be verified, we might be vulnerable.

  • CVE-2022-25314 was introduced in 2.2.2, which we haven't updated to.

  • CVE-2022-25315 doesn't affect us, we define XML_UNICODE.

(In reply to Olli Pettay [:smaug] from comment #2)

Bobby, do you think RLBoxing expat on ESR was feasible?
(as a possible alternative to updating expat)

There was a bunch of rlbox infrastructure that landed over the summer, so my guess is that cherry-picking expat fixes will be less work than trying to uplift the expat sandbox.

Flags: needinfo?(bholley)

Apparently glandium has patches already to uplift RLBox to ESR, so that might in fact happen anyway.

Assignee: nobody → htsai

(In reply to Peter Van der Beken [:peterv] from comment #3)

Peter assessed that these are simple fixes that we should just cherry-pick.

  • CVE-2022-23852: doesn't affect us, only affects "configurations with a nonzero XML_CONTEXT_BYTES", we don't set XML_CONTEXT_BYTES to nonzero.

  • CVE-2022-23990: on first glance I don't think we're affected: the patch has a comment saying "when there is an element declaration handler present (from a prior call to XML_SetElementDeclHandler)". We never set an element declaration handler.

  • CVE-2022-25236: on first glance I don't think we're affected: our namespace separator (0xFFFF) shouldn't make it into the data being parsed in the first place.

  • CVE-2022-25313 needs to be verified, we might be vulnerable.

  • CVE-2022-25314 was introduced in 2.2.2, which we haven't updated to.

  • CVE-2022-25315 doesn't affect us, we define XML_UNICODE.

Assignee: htsai → peterv

Cool, will it make it to the 91.7 esr? Thanks.

Flags: needinfo?(peterv)

(In reply to Jan Horak [:jhorak] from comment #7)

Cool, will it make it to the 91.7 esr? Thanks.

No, that candidate was already created.

Flags: needinfo?(peterv)

Depends on D140167

What about the CVE-2022-25235?

Attachment #9266237 - Attachment is obsolete: true

(In reply to Jan Horak [:jhorak] from comment #13)

What about the CVE-2022-25235?

We don't use Expat in utf-8 mode, so I don't think that affects us.

Comment on attachment 9266234 [details]
Bug 1754724 - Clear up some computations in expat code. r?farre!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Probably, yes.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • 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?: Patches should apply as-is.
  • How likely is this patch to cause regressions; how much testing does it need?: Patches look low-risk, they just add overflow checks in various places.
Attachment #9266234 - Flags: sec-approval?
Attachment #9266235 - Flags: sec-approval?
Attachment #9266236 - Flags: sec-approval?
Attachment #9266234 - Flags: sec-approval? → sec-approval+
Attachment #9266235 - Flags: sec-approval? → sec-approval+
Attachment #9266236 - Flags: sec-approval? → sec-approval+

Comment on attachment 9266234 [details]
Bug 1754724 - Clear up some computations in expat code. r?farre!

Adding 98.0rc3 & 91.7esr RC2 branch approvals to go with Tom's sec-approval.

Attachment #9266234 - Flags: approval-mozilla-release+
Attachment #9266234 - Flags: approval-mozilla-esr91+
Attachment #9266235 - Flags: approval-mozilla-release+
Attachment #9266235 - Flags: approval-mozilla-esr91+
Attachment #9266236 - Flags: approval-mozilla-release+
Attachment #9266236 - Flags: approval-mozilla-esr91+

Should we call these out in the advisories for 98/91.7esr?

Flags: needinfo?(tom)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(peterv)
Whiteboard: sec-high for ESR which has no RLBox → sec-high for ESR which has no RLBox[sec-survey]

Dan and I's reading of this was that we don't think any of the issues apply to us, and we took them to be safe, so we decided they don't need to be included.

Flags: needinfo?(tom)
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Group: core-security-release

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #21)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

The form is no longer accepting responses.

Flags: needinfo?(peterv)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: