Closed Bug 798802 Opened 7 years ago Closed 7 years ago

mixing webgl and 2d context causes crashes

Categories

(Core :: Canvas: 2D, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox17 --- unaffected
firefox18 + fixed
firefox19 + fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected

People

(Reporter: miaubiz, Assigned: ajones)

References

Details

(Keywords: csectype-uninitialized, regression, sec-critical, Whiteboard: [asan][adv-main18+])

Attachments

(4 files, 1 obsolete file)

Attached file repro case
I load:

<html>
  <head>
    <script>
      onload = function() {
        var canvas2d = document.createElement('canvas')
        canvas2d.setAttribute('width', 0)
        document.body.appendChild(canvas2d)
        var ctx2d = canvas2d.getContext('2d')
        ctx2d.fillStyle = 'black'
        var gl = document.createElement('canvas').getContext('experimental-webgl')
        gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, gl.RGBA, gl.UNSIGNED_BYTE, canvas2d)
        ctx2d.fillRect(0, 0, 1, 1)
      }
    </script>
  </head>
  <body>
  </body>
</html>


and I get:

ASAN:SIGSEGV
==860== ERROR: AddressSanitizer crashed on unknown address 0x0096000001d4 (pc 0x7fffee78f0d8 sp 0x7fffffff4040 bp 0x7fffffff4130 T0)
AddressSanitizer can not provide additional info. ABORTING
    #0 0x7fffee78f0d7 in mozilla::layers::CanvasLayer::Updated() /builds/slave/try-lnx64/build/../../../../dist/include/Layers.h:1390
    #1 0x7fffee5bd01e in nsCanvasRenderingContext2DAzure::Redraw(mozilla::gfx::Rect const&) /builds/slave/try-lnx64/build/content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:821
    #2 0x7fffee5cb9ae in nsCanvasRenderingContext2DAzure::RedrawUser(gfxRect const&) 

on osx I get:

==19386== ERROR: AddressSanitizer crashed on unknown address 0x0096000001dc (pc 0x000105d1b34e sp 0x7fff5fbf2670 bp 0x7fff5fbf2670 T0)
AddressSanitizer can not provide additional info. ABORTING
    #0 0x105d1b34d in mozilla::layers::CanvasLayer::Updated() (in XUL) + 45
    #1 0x105e978d4 in nsHTMLCanvasElement::InvalidateCanvasContent(gfxRect const*) (in XUL) + 516
    #2 0x105d24d80 in nsCanvasRenderingContext2DAzure::Redraw(mozilla::gfx::Rect const&) (in XUL) + 624
    #3 0x105d24fa1 in nsCanvasRenderingContext2DAzure::RedrawUser(gfxRect const&) (in XUL) + 433


altering the width/height of the 2d context changes the segfaulty address.
Attached file asan log linux
Attached file asan log osx
Attachment #668797 - Attachment mime type: text/plain → text/html
Attachment #669362 - Flags: review? → review?(matt.woodrow)
Comment on attachment 669362 [details] [diff] [review]
Part 1: Fix layer uninitialised in nsHTMLCanvasElement::InvalidateCanvasContent()

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

Just setting layer = nullptr initially might be easier.
Attachment #669362 - Flags: review?(matt.woodrow) → review+
As per suggestion and with crashtest.
Attachment #669387 - Flags: review?(matt.woodrow)
Attachment #669387 - Flags: review?(matt.woodrow) → review+
Attachment #669362 - Attachment is obsolete: true
Does this bug need a sec-rating and sec-approval?
mccr8 suspects this is a regression from bug 539356 and therefore won't affect Firefox 17 or earlier.
(In reply to Daniel Veditz [:dveditz] from comment #8)
> mccr8 suspects this is a regression from bug 539356 and therefore won't
> affect Firefox 17 or earlier.

Does this mean it doesn't need approval to land? I'm still a bit fuzzy on how the new process works.
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #9)
> Is this the same crash?
> https://crash-stats.mozilla.com/report/index/bp-2f5faa8d-9ac3-4270-af78-
> 7f4e62121008

No. That crash looks like presContext->PresShell() is returning nullptr. presContext is checked above.
Ok, filed bug 800147 for it, then.
Comment on attachment 669387 [details] [diff] [review]
Fix layer uninitialised in nsHTMLCanvasElement::InvalidateCanvasContent() v2

[Security approval request comment]
How easily can the security issue be deduced from the patch?

Yes.


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

Yes. The patch includes a crash test.


Which older supported branches are affected by this flaw?

Firefox 18.

If not all supported branches, which bug introduced the flaw?

Bug 539356 https://hg.mozilla.org/mozilla-central/rev/6cabf68e297b

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

It is easy to back port.

How likely is this patch to cause regressions; how much testing does it need?

This patch itself is very unlikely to cause regressions.
Attachment #669387 - Flags: sec-approval?
Comment on attachment 669387 [details] [diff] [review]
Fix layer uninitialised in nsHTMLCanvasElement::InvalidateCanvasContent() v2

sec-approval+

Since this does not affect release please go ahead and check in the tests, and make sure to request approval to land on the affected branch!
Attachment #669387 - Flags: sec-approval? → sec-approval+
Comment on attachment 669387 [details] [diff] [review]
Fix layer uninitialised in nsHTMLCanvasElement::InvalidateCanvasContent() v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 539356 commit 6cabf68e297b
User impact if declined: crash if mixing 2d and 3d content
Testing completed (on m-c, etc.): None.
Risk to taking this patch (and alternatives if risky): Very low risk.
String or UUID changes made by this patch: None.
Attachment #669387 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e05e9c4666e1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Attachment #669387 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [adv-main18+]
Whiteboard: [adv-main18+] → [asan][adv-main18+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.