Closed Bug 1797370 Opened 1 year ago Closed 1 year ago

Crash [@ nsJARChannel::OnStartRequest] EXEC and WRITE with wildptr addresses

Categories

(Core :: Networking: JAR, defect, P2)

defect

Tracking

()

RESOLVED FIXED
109 Branch
Tracking Status
firefox-esr102 108+ fixed
firefox107 --- wontfix
firefox108 + fixed
firefox109 + fixed

People

(Reporter: jesup, Assigned: valentin)

References

Details

(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main108+r][adv-esr102.6+r])

Crash Data

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #237206 +++

Quite likely this is a different bug than the original one with this signature.

Group: core-security → network-core-security
Severity: -- → S2
Priority: -- → P2
Assignee: nobody → valentin.gosu

I'm still looking into this.
I'm thinking this is some kind of race in nsInputStreamPump, maybe with off main thread delivery?

Whiteboard: [startupcrash][necko-triaged][necko-priority-review] → [startupcrash][necko-triaged][necko-priority-queue]

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not trivial. It's still not very clear how one would have caused the listener to change while the mutex was unlocked.
  • 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?: Yes
  • If not, how different, hard to create, and risky will they be?:
  • How likely is this patch to cause regressions; how much testing does it need?: Low chance of regressions. Side effects would be an extra addref for each OnStart/Stop/Data call.
  • Is Android affected?: Yes
Attachment #9303670 - Flags: sec-approval?

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

Approved to land and request uplift

Attachment #9303670 - Flags: sec-approval? → sec-approval+
Group: network-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

Beta/Release Uplift Approval Request

  • User impact if declined: Possible UAF
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Avoids using mutex protected variable while the mutex is unlocked by putting variable into nsCOMPtr before unlocking. There should be no behaviour change at all.
  • String changes made/needed:
  • Is Android affected?: Unknown
Attachment #9303670 - Flags: approval-mozilla-beta?

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

Approved for 108.0b7

Attachment #9303670 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Please nominate this for ESR102 approval also.

Flags: needinfo?(valentin.gosu)

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Possible UAF
  • Fix Landed on Version: 109
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Avoids using mutex protected variable while the mutex is unlocked by putting variable into nsCOMPtr before unlocking. There should be no behaviour change at all.
Flags: needinfo?(valentin.gosu)
Attachment #9303670 - Flags: approval-mozilla-esr102?

Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup

Approved for 102.6esr.

Attachment #9303670 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+
Flags: qe-verify-
Whiteboard: [startupcrash][necko-triaged][necko-priority-queue] → [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage]
Whiteboard: [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage] → [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main108+r]
Whiteboard: [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main108+r] → [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main108+r][adv-esr102.6+r]
See Also: → 1732885
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: