Upgrade AssertIsOnTargetThread() in EventSource.cpp to MOZ_DIAGNOSTIC_ASSERT

RESOLVED FIXED in Firefox 64

Status

()

defect
P2
normal
RESOLVED FIXED
11 months ago
13 days ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

({sec-other})

unspecified
mozilla64
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main64-])

Attachments

(1 attachment)

Blocks: 1485722
MozReview-Commit-ID: 9e5vbMW07eH
Group: core-security → dom-core-security
Priority: -- → P2
Keywords: sec-other
Comment on attachment 9015207 [details]
Bug 1497160 - Upgrade assertion type.

[Security Approval Request]

How easily could an exploit be constructed based on the patch?: The patch hints at a threading problem in this area, but doesn't say what the problem is. (I don't know, either.)

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?: Bug 1267903

Do you have backports for the affected branches?: No

If not, how different, hard to create, and risky will they be?: This is a diagnostic patch, so I don't expect an uplift.

How likely is this patch to cause regressions; how much testing does it need?: The purpose is to turn some non-crashes into crashes on Nightly and Dev Edition in order to track down the real bug, so in that sense the hope is more crash stacks.
Attachment #9015207 - Flags: sec-approval?
Comment on attachment 9015207 [details]
Bug 1497160 - Upgrade assertion type.

Sec-approval=dveditz to land this diagnostic patch.
Attachment #9015207 - Flags: sec-approval? → sec-approval+
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
No crash reports yet AFAICT.
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Still no crash reports with AssertIsOnTargetThread on the stack.
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main64-]

In the absence of nightly or dev edition crash reports, should we try making it a release assert at least temporarily in the hope it fires on beta?

Component: DOM → DOM: Core & HTML
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.