1558009 - Crash in [@ mozilla::dom::CanvasRenderingContext2D::GetImageBuffer]

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Calixte Denizet (:calixte)]

This bug is for crash report bp-9e91cb45-c74a-4263-a9c0-c51800190608.

Top 10 frames of crashing thread:

0 xul.dll class mozilla::UniquePtr<unsigned char [], mozilla::DefaultDelete<unsigned char []> > mozilla::dom::CanvasRenderingContext2D::GetImageBuffer dom/canvas/CanvasRenderingContext2D.cpp:1620
1 xul.dll nsresult mozilla::dom::CanvasRenderingContext2D::GetInputStream dom/canvas/CanvasRenderingContext2D.cpp:1657
2 xul.dll static nsresult mozilla::dom::ImageEncoder::ExtractDataInternal dom/base/ImageEncoder.cpp:325
3 xul.dll mozilla::dom::ImageEncoder::ExtractData dom/base/ImageEncoder.cpp:229
4 xul.dll nsresult mozilla::dom::HTMLCanvasElement::ExtractData dom/html/HTMLCanvasElement.cpp:723
5 xul.dll nsresult mozilla::dom::HTMLCanvasElement::ToDataURLImpl dom/html/HTMLCanvasElement.cpp:751
6 xul.dll mozilla::dom::HTMLCanvasElement::ToDataURL dom/html/HTMLCanvasElement.cpp:604
7 xul.dll static bool mozilla::dom::HTMLCanvasElement_Binding::toDataURL dom/bindings/HTMLCanvasElementBinding.cpp:357
8 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3171
9 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:540

There is 1 crash in nightly 69 with buildid 20190608094203. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1464032.

[1] https://hg.mozilla.org/mozilla-central/rev?node=4357d695b8d5

Comment 1

[Bob Owen (:bobowen)]

The only way that I can see that EnsureTarget returns true, without having an mBufferProvider is at [1].
Also, the only place I can see where we could have mTarget, but not mBufferProvider is at [2].

So I guess we should add a check at [1] for mTarget == sErrorTarget.
The question is mTarget == sErrorTarget considered fatal for the CanvasRenderingContext2D, in which case I guess we return false or can it be recovered from in which case we would continue and attempt to create a new provider and target.

[1] https://searchfox.org/mozilla-central/rev/ee806041c6f76cc33aa3c9869107ca87cb3de371/dom/canvas/CanvasRenderingContext2D.cpp#1229-1230
[2] https://searchfox.org/mozilla-central/rev/ee806041c6f76cc33aa3c9869107ca87cb3de371/dom/canvas/CanvasRenderingContext2D.cpp#1357-1358

Comment 2

[Bob Owen (:bobowen)]

Looks like this is a painful issue for fuzzing, so having re-looked at the original code I think that we should return false if mTarget == sErrorTarget:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e27d06136f1cc14444ef39ce4d0b8c7d51bbf65c

Comment 3

[Bob Owen (:bobowen)]

Attachment: Bug 1558009: Always return false from CanvasRenderingContext2D::EnsureTarget when mTarget == sErrorTarget. r=jrmuizel!

Comment 4

[Bob Owen (:bobowen)]

Confirmed in bug 1558268 comment 6 that this patch fixes the problem for the fuzzing.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88ac974e3453
Always return false from CanvasRenderingContext2D::EnsureTarget when mTarget == sErrorTarget. r=mattwoodrow

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/88ac974e3453