Closed Bug 1330513 Opened 8 years ago Closed 8 years ago

Null-deref in [@ GetUnit] with Animation options composite: add and fill: forwards

Categories

(Core :: DOM: Animation, defect, P1)

Product:
Core
Component:
DOM: Animation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- fixed

People

(Reporter: truber, Assigned: hiro)

References

(Blocks 1 open bug)

Details

(Keywords: crash, csectype-nullptr, testcase)

Attachments

(3 files)

testcase.html
148 bytes, text/html
Details
log.txt
8.32 KB, text/plain
Details
Bug 1330513- Check that mCSSValueList is null in case of filter and shadow in StyleAnimationValue::Add().
59 bytes, text/x-review-board-request
boris
: review+
Details
Attached file testcase.html —
The attached testcase causes a nullptr dereference in mozilla-central rev 63ad56438630. ==12848==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f9f9b3c2495 bp 0x7ffe5b212630 sp 0x7ffe5b212540 T0) #0 0x7f9f9b3c2494 in GetUnit /home/worker/workspace/build/src/layout/style/nsCSSValue.h:629:38 #1 0x7f9f9b3c2494 in mozilla::StyleAnimationValue::Add(nsCSSPropertyID, mozilla::StyleAnimationValue const&, mozilla::StyleAnimationValue&&) /home/worker/workspace/build/src/layout/style/StyleAnimationValue.cpp:787 #2 0x7f9f973837c0 in mozilla::dom::KeyframeEffectReadOnly::CompositeValue(nsCSSPropertyID, mozilla::StyleAnimationValue const&, mozilla::StyleAnimationValue const&, mozilla::dom::CompositeOperation) /home/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:333:14 #3 0x7f9f97383e51 in mozilla::dom::KeyframeEffectReadOnly::CompositeValue(nsCSSPropertyID, RefPtr<mozilla::AnimValuesStyleRule> const&, mozilla::StyleAnimationValue const&, mozilla::dom::CompositeOperation) /home/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:395:10 #4 0x7f9f9735a050 in mozilla::dom::KeyframeEffectReadOnly::ComposeStyle(RefPtr<mozilla::AnimValuesStyleRule>&, nsCSSPropertyIDSet const&) /home/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:524:7
Attached file log.txt —
Hmm, the StyleAnimationValue for the filter has a valid unit, eUnit_Filter, but mCSSValueList is nullptr.
Flags: needinfo?(hikezoe)
See Also: → 1330190
Blocks: 1311620
Flags: needinfo?(hikezoe)
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Priority: -- → P1
Comment on attachment 8826058 [details] Bug 1330513- Check that mCSSValueList is null in case of filter and shadow in StyleAnimationValue::Add(). https://reviewboard.mozilla.org/r/104102/#review104840 ::: dom/animation/test/crashtests/1330513-1.html:5 (Diff revision 1) > +<!DOCTYPE html> > +<html> > +<body id=a></body> > +<script> > +a.animate([{"filter": "grayscale(28%)"}], {fill:"forwards", composite:"add"}); Don't we need to call document.getElementById('a') first to get the body element? ::: dom/animation/test/crashtests/crashtests.list:19 (Diff revision 1) > pref(dom.animations-api.core.enabled,true) load 1304886-1.html > pref(dom.animations-api.core.enabled,true) load 1322382-1.html > skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-1.html # bug 1323733 > asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-1.html # bug 1324690 > asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-2.html # bug 1324690 > +skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330513-1.html # bug 1311257 r/1311257/1330513
Attachment #8826058 - Flags: review?(boris.chiou) → review+
(In reply to Boris Chiou [:boris] from comment #5) > Comment on attachment 8826058 [details] > Bug 1330513- Check that mCSSValueList is null in case of filter and shadow > in StyleAnimationValue::Add(). > > https://reviewboard.mozilla.org/r/104102/#review104840 > > ::: dom/animation/test/crashtests/1330513-1.html:5 > (Diff revision 1) > > +<!DOCTYPE html> > > +<html> > > +<body id=a></body> > > +<script> > > +a.animate([{"filter": "grayscale(28%)"}], {fill:"forwards", composite:"add"}); > > Don't we need to call document.getElementById('a') first to get the body > element? It's named access on the window object. https://html.spec.whatwg.org/#named-access-on-the-window-object But I will replace it with getElementById(). > ::: dom/animation/test/crashtests/crashtests.list:19 > (Diff revision 1) > > pref(dom.animations-api.core.enabled,true) load 1304886-1.html > > pref(dom.animations-api.core.enabled,true) load 1322382-1.html > > skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-1.html # bug 1323733 > > asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-1.html # bug 1324690 > > asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-2.html # bug 1324690 > > +skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330513-1.html # bug 1311257 > > r/1311257/1330513 The comment is for skip-if(stylo), that means we will drop the skip-if once bug 1311257 is fixed. Thanks for the quick review!
Pushed by hiikezoe@mozilla-japan.org: https://hg.mozilla.org/integration/autoland/rev/0a4f47afa494 Check that mCSSValueList is null in case of filter and shadow in StyleAnimationValue::Add(). r=boris
https://hg.mozilla.org/mozilla-central/rev/0a4f47afa494
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: