JSD.pause and unPause can attempt to toggle debug traps on non-debug mode Baseline scripts

RESOLVED FIXED in Firefox 27

Status

()

defect
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: shu, Assigned: shu)

Tracking

({sec-moderate})

unspecified
mozilla28
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox25 wontfix, firefox26 wontfix, firefox27 fixed, firefox28 fixed, firefox-esr24 wontfix, b2g18 unaffected, b2g-v1.2 unaffected)

Details

(Whiteboard: [adv-main27-] exploit requires Firebug plus a malicious addon that uses Firebug)

Attachments

(2 attachments)

Assignee

Description

6 years ago
JSD.pause and unPause promise to persist any breakpoints set on JSScripts [1], but we need to invalidate JIT code when toggling debug mode. This can get into a inconsistent state where we try to patch toggled calls on a non-debug mode BaselineScript.

Setting s-s because the patching is only guarded by an assert, and it could corrupt the instruction stream.
Assignee

Updated

6 years ago
Blocks: 815603
Assignee

Comment 2

6 years ago
We need to check this because something like JSCompartment::clearTraps tries to
toggle debug traps if hasBaselineScript(), without regard for whether or not
that BL script is in debug mode.
Attachment #827821 - Flags: review?(jdemooij)
Assignee

Updated

6 years ago
Assignee: nobody → shu
Status: NEW → ASSIGNED
Comment on attachment 827821 [details] [diff] [review]
Toggle debug traps only in debug mode. (r=?)

Review of attachment 827821 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.
Attachment #827821 - Flags: review?(jdemooij) → review+
Keywords: sec-high
https://hg.mozilla.org/mozilla-central/rev/fe63bf2c6a3a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
sec-high and no security approval?
Target Milestone: --- → mozilla28
I'm going to guess this affects everything aside from b2g18.
Assignee

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 10

6 years ago
Is this sec-high if it can only be caused by Firebug?
I was using the heuristic of bumping it down a level (from sec-critical) because it is Firebug related.

Is this something that can happen during the normal operation of Firebug, or something that would require a malicious Firebug-like addon?
Assignee

Comment 14

6 years ago
(In reply to Andrew McCreight [:mccr8] from comment #11)
> I was using the heuristic of bumping it down a level (from sec-critical)
> because it is Firebug related.
> 
> Is this something that can happen during the normal operation of Firebug, or
> something that would require a malicious Firebug-like addon?

It would require a malicious addon that uses JSD. Which... is nonexistent AFAIK.
Assignee

Comment 15

6 years ago
Comment on attachment 8341334 [details] [diff] [review]
Toggle debug traps only in debug mode. (

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
  Bug 754201 most recently. Not sure what introduced AutoDebugModeGC initially.

User impact if declined: 
  Firebug unusable with many tabs. Firebug has worked around this by disabling JITs permanently if it's on, which is in some ways worse since it slows down *all* tabs.

Testing completed (on m-c, etc.): 
  On m-c.

Risk to taking this patch (and alternatives if risky): 
  GC behavior changes surfacing existing leaks/OOMs. When landing on m-c new leaks were surfaced (and eventually fixed). I am nominating those fixes for uplift also.

String or IDL/UUID changes made by this patch:
  None

The Firebug slowness metabug is bug 815603. List of bugs I'm nominating for uplift in this batch:

 - bug 936143
 - bug 935228
 - bug 933882
 - bug 935470 
 - bug 942346
 - bug 934799
Attachment #8341334 - Flags: approval-mozilla-beta?
Attachment #8341334 - Flags: approval-mozilla-aurora?
Thanks for the explanation.  sec-moderate makes more sense then.
Keywords: sec-highsec-moderate
Comment on attachment 8341334 [details] [diff] [review]
Toggle debug traps only in debug mode. (

We can take this on Aurora and thus it will be on Beta in a week, but we're way past the point of taking untracked, major change to Firefox 26 as we are going to build our final candidate today as well as the RC.
Attachment #8341334 - Flags: approval-mozilla-beta?
Attachment #8341334 - Flags: approval-mozilla-beta-
Attachment #8341334 - Flags: approval-mozilla-aurora?
Attachment #8341334 - Flags: approval-mozilla-aurora+
Shu, IIUC this can be closed, right?
Flags: needinfo?(shu)
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Assignee

Comment 20

6 years ago
Actually I'm not sure why it stayed open anyways, because it was REOPENED?
Flags: needinfo?(shu)
(In reply to Shu-yu Guo [:shu] from comment #20)
> Actually I'm not sure why it stayed open anyways, because it was REOPENED?

Security bugs are resolved manually. He probably just forgot.
Whiteboard: [adv-main27-]
Whiteboard: [adv-main27-] → [adv-main27-] exploit requires Firebug plus a malicious addon that uses Firebug
Group: core-security
You need to log in before you can comment on or make changes to this bug.