The default bug view has changed. See this FAQ.

Crash with <svg:use> removed during its own scrollbar attribute mutation event

RESOLVED FIXED

Status

()

Core
Layout
--
critical
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: smaug)

Tracking

(Blocks: 2 bugs, 5 keywords)

17 Branch
crash, csectype-uaf, regression, sec-critical, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox16 wontfix, firefox17+ fixed, firefox18+ fixed, firefox19+ fixed, firefox-esr1017+ fixed)

Details

(Whiteboard: [adv-track-main17+][adv-track-esr17+] The current testcase is only useful on recent versions, crash signature)

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 664226 [details]
testcase (crashes Firefox when loaded)

bp-0cada683-3e71-482d-8d98-0c2d92120924

I'm guessing this is a regression from bug 778810, which added the function mozilla::ScrollbarActivity::ActivityOccurred.

Why does scrolling set DOM attributes?
(Reporter)

Comment 1

5 years ago
Created attachment 664227 [details]
stack traces: (1) mutation event listener, (2) crash
(Assignee)

Comment 2

5 years ago
Why do we end up dispatching mutation event. Scrollbars should be native anonymous.

Updated

5 years ago
Keywords: regression
Version: Trunk → 17 Branch
(Assignee)

Updated

5 years ago
Assignee: nobody → bugs
Group: core-security
(Assignee)

Comment 3

5 years ago
Created attachment 664662 [details] [diff] [review]
patch

Shouldn't rely on the element type, but frame type.
In the test we end up getting parent frame which is ScrollFrame.
Attachment #664662 - Flags: review?(bzbarsky)
status-firefox-esr10: --- → unaffected
status-firefox16: --- → unaffected
status-firefox17: --- → affected
status-firefox18: --- → affected
tracking-firefox17: --- → +
tracking-firefox18: --- → +
Keywords: csec-uaf, sec-critical
(Assignee)

Comment 4

5 years ago
As far as I see, this is a really old bug.
status-firefox15: --- → affected
status-firefox16: unaffected → affected
Keywords: regression

Comment 5

5 years ago
I loaded the testcase on Windows:
* 15.0.1: no crash
* 16.0b4: no crash
* 17.0a2: bp-101eb84b-d2b6-4f41-9dbf-37a5c2120926
* 18.0a1: bp-5894bccb-d4ac-448a-b32b-4c32d2120926
Crash Signature: [@ mozilla::ScrollbarActivity::ActivityOccurred] → [@ mozilla::ScrollbarActivity::ActivityOccurred] [@ mozilla::ScrollbarActivity::CancelActivityFinishedTimer()] [@ nsListScrollSmoother::Stop()]
status-firefox15: affected → unaffected
status-firefox16: affected → unaffected
Keywords: regression
OS: Mac OS X → All
Hardware: x86_64 → All
(Assignee)

Comment 6

5 years ago
Sure, using *that* testcase. But I don't see why the problem wouldn't happen with some other
testcase too.
Comment on attachment 664662 [details] [diff] [review]
patch

r=me
Attachment #664662 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 8

5 years ago
Comment on attachment 664662 [details] [diff] [review]
patch

[Security approval request comment]
How easily can the security issue be deduced from the patch?
I'd say quite easily. Just try to make svg:use to use some unusual frame type

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
NA

Which older supported branches are affected by this flaw?
All

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The same patch should work in older branches too.
(if the patch doesn't apply cleanly, the changes should be minor
syntactic changes. The code is several years old.)

How likely is this patch to cause regressions; how much testing does it need?
Should be reasonable safe. In normal cases svg:use should behave like svg:use.
Attachment #664662 - Flags: sec-approval?
Attachment #664662 - Flags: approval-mozilla-esr10?
Attachment #664662 - Flags: approval-mozilla-beta?
Attachment #664662 - Flags: approval-mozilla-aurora?
Comment on attachment 664662 [details] [diff] [review]
patch

sec-approval+ assuming you can get branch landing approval from drivers
Attachment #664662 - Flags: sec-approval? → sec-approval+
(In reply to Olli Pettay [:smaug] from comment #8)
> Which older supported branches are affected by this flaw?
> All

Even though we don't crash in 15/16? Why are those branches marked as unaffected?

We're leaning towards not taking this for FF16 to limit risk, since this was internally reported.
(Assignee)

Comment 11

5 years ago
So I can certainly get access to supposed-to-be-native-anonymous element in FF16. So far no
crash, but I don't see why.
(Assignee)

Comment 12

5 years ago
If we don't take this to FF16, should we land the patch closer to FF17 release?
Comment on attachment 664662 [details] [diff] [review]
patch

Longstanding issue, internally reported, and FF16 doesn't crash. I don't think this qualifies for our final beta. Setting the sec-approval flag back to ? to answer the question of when to land on Central/Aurora/(next Beta).
Attachment #664662 - Flags: sec-approval?
Attachment #664662 - Flags: sec-approval+
Attachment #664662 - Flags: approval-mozilla-esr10?
Attachment #664662 - Flags: approval-mozilla-esr10-
Attachment #664662 - Flags: approval-mozilla-beta?
Attachment #664662 - Flags: approval-mozilla-beta-
Attachment #664662 - Flags: approval-mozilla-aurora?
Attachment #664662 - Flags: approval-mozilla-aurora-
status-firefox15: unaffected → ---
status-firefox16: unaffected → wontfix
Whiteboard: Wait until Oct 21 to land
We'll want a test for this that we can check in once we've release Fx17.
status-firefox-esr10: unaffected → affected
status-firefox19: --- → affected
tracking-firefox-esr10: --- → 17+
tracking-firefox19: --- → +
Flags: in-testsuite?
Keywords: testcase-wanted
Whiteboard: Wait until Oct 21 to land → The current testcase is only useful on recent versions
Comment on attachment 664662 [details] [diff] [review]
patch

sec-approval+, re-requesting branch approvals.
Attachment #664662 - Flags: sec-approval?
Attachment #664662 - Flags: sec-approval+
Attachment #664662 - Flags: approval-mozilla-esr10?
Attachment #664662 - Flags: approval-mozilla-esr10-
Attachment #664662 - Flags: approval-mozilla-beta?
Attachment #664662 - Flags: approval-mozilla-beta-
Attachment #664662 - Flags: approval-mozilla-aurora?
Attachment #664662 - Flags: approval-mozilla-aurora-
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ae051b7f7e4e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
status-firefox19: affected → fixed
Target Milestone: --- → mozilla19
Attachment #664662 - Flags: approval-mozilla-esr10?
Attachment #664662 - Flags: approval-mozilla-esr10+
Attachment #664662 - Flags: approval-mozilla-beta?
Attachment #664662 - Flags: approval-mozilla-beta+
Attachment #664662 - Flags: approval-mozilla-aurora?
Attachment #664662 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/e77259feea47
https://hg.mozilla.org/releases/mozilla-aurora/rev/dcc60baeca75
https://hg.mozilla.org/releases/mozilla-esr10/rev/92ef4d0a9c88
status-firefox-esr10: affected → fixed
status-firefox17: affected → fixed
status-firefox18: affected → fixed
Target Milestone: mozilla19 → ---
Whiteboard: The current testcase is only useful on recent versions → [adv-track-main17+][adv-track-esr17+] The current testcase is only useful on recent versions
Keywords: verifyme
Group: core-security
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb0da62e1348
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/eb0da62e1348
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Keywords: testcase-wanted
You need to log in before you can comment on or make changes to this bug.