Last Comment Bug 815795 - (CVE-2013-0768) stack buffer overflow with canvas
(CVE-2013-0768)
: stack buffer overflow with canvas
Status: RESOLVED FIXED
[asan][adv-main18+][adv-esr17+]
: csectype-intoverflow, sec-critical
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla20
Assigned To: George Wright (:gw280) (:gwright)
:
Mentors:
Depends on:
Blocks: 826379
  Show dependency treegraph
 
Reported: 2012-11-27 13:47 PST by miaubiz
Modified: 2014-07-24 14:37 PDT (History)
18 users (show)
abillings: sec‑bounty+
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
fixed
+
fixed
unaffected
18+
fixed
fixed


Attachments
repro case (823 bytes, text/plain)
2012-11-27 13:47 PST, miaubiz
no flags Details
asan log linux (2.20 KB, text/plain)
2012-11-27 13:47 PST, miaubiz
no flags Details
Fix integer overflow (861 bytes, patch)
2012-12-19 15:14 PST, George Wright (:gw280) (:gwright)
no flags Details | Diff | Review
Fix integer overflow (957 bytes, patch)
2012-12-19 15:15 PST, George Wright (:gw280) (:gwright)
jmuizelaar: review+
bajaj.bhavana: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
bajaj.bhavana: approval‑mozilla‑esr10+
bajaj.bhavana: approval‑mozilla‑esr17+
abillings: sec‑approval+
Details | Diff | Review

Description miaubiz 2012-11-27 13:47:00 PST
Created attachment 685817 [details]
repro case

<html>
  <head>
    <script>
        setTimeout("location.reload()", 1000)
        el18=document.createElement('canvas')
        ctx=el18.getContext('2d')
        ctx.shadowColor = 'black'
        ctx.shadowBlur = 8
        ctx.globalAlpha=0
        ctx.miterLimit=0
        ctx.strokeStyle=ctx.createLinearGradient(0, 0, 0, 0)
        ctx.beginPath()
        ctx.arc(50, 80, 125, 0.1, -1, false)
        ctx.scale(0.0055, 1)
        document.implementation.createDocument('' ,'' ,null).adoptNode(el18)
        var imgxz= new Image()
        imgxz.src = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=='
        ctx.fillStyle=ctx.createPattern(imgxz,'')
        ctx.fill('', 0, 0)
    </script>
  </head>
  <body>
  </body>
</html>

results in:

==3262== ERROR: AddressSanitizer stack-buffer-overflow on address 0x7fffffff2c9c at pc 0x7ffff0388727 bp 0x7fffffff2990 sp 0x7fffffff2988
READ of size 4 at 0x7fffffff2c9c thread T0
    #0 0x7ffff0388726 in fast_composite_scaled_bilinear_sse2_8888_8_8888_normal_OVER /builds/slave/try-lnx64/build/gfx/cairo/libpixman/src/pixman-sse2.c:5725
    #1 0x7ffff02d8d97 in _moz_pixman_image_composite32 /builds/slave/try-lnx64/build/gfx/cairo/libpixman/src/pixman.c:712
    #2 0x7ffff026227e in _composite_spans /builds/slave/try-lnx64/build/gfx/cairo/cairo/src/cairo-image-surface.c:3496
Address 0x7fffffff2c9c is located at offset 220 in frame <fast_composite_scaled_bilinear_sse2_8888_8_8888_normal_OVER> of T0's stack:
  This frame has 5 object(s):
    [32, 44) 'v'
    [96, 104) 'buf1'
    [160, 168) 'buf2'
    [224, 736) 'extended_src_line0'
    [768, 1280) 'extended_src_line1'
Comment 1 miaubiz 2012-11-27 13:47:25 PST
Created attachment 685819 [details]
asan log linux
Comment 2 David Bolter [:davidb] 2012-12-12 08:21:06 PST
cc+ Milan. Needs assignee and a sec rating.
Comment 3 Milan Sreckovic [:milan] 2012-12-12 08:34:04 PST
George, can you take a quick look?
Comment 4 George Wright (:gw280) (:gwright) 2012-12-17 23:43:33 PST
Well, it's certainly reproducible. I'm having trouble debugging it right now, but I'm getting closer. Pixman's extensive use of preprocessor functions is causing me grief :)

This is a partial stacktrace of what I've got so far:

(gdb) bt
#0  0x0000000000429cb0 in __asan_report_error ()
#1  0x000000000042a0c7 in __asan_report_load4 ()
#2  0x00007fffef82786f in fast_composite_scaled_bilinear_sse2_8888_8_8888_normal_OVER (imp=<optimized out>, 
    info=<error reading variable: Unhandled dwarf expression opcode 0x0>) at /home/george/dev/mozilla-central-2/gfx/cairo/libpixman/src/pixman-sse2.c:26851
#3  0x00007fffef78e292 in _moz_pixman_image_composite32 (op=<optimized out>, src=<error reading variable: Unhandled dwarf expression opcode 0x0>, mask=<optimized out>, 
    dest=<optimized out>, src_x=<optimized out>, src_y=<optimized out>, mask_x=0, mask_y=0, dest_x=<optimized out>, dest_y=<optimized out>, width=<optimized out>, 
    height=<optimized out>) at /home/george/dev/mozilla-central-2/gfx/cairo/libpixman/src/pixman.c:712
#4  0x00007fffef68caf3 in _composite_spans (closure=<error reading variable: Unhandled dwarf expression opcode 0xa9>, dst=<optimized out>, dst_format=<optimized out>, 
    op=<optimized out>, pattern=<optimized out>, dst_x=<optimized out>, dst_y=<optimized out>, extents=<error reading variable: Unhandled dwarf expression opcode 0x1>, 
    clip_region=0x0) at /home/george/dev/mozilla-central-2/gfx/cairo/cairo/src/cairo-image-surface.c:3496
#5  0x00007fffef680950 in _clip_and_composite (dst=<optimized out>, op=<optimized out>, src=<error reading variable: Unhandled dwarf expression opcode 0x0>, 
    draw_func=<optimized out>, draw_closure=<optimized out>, extents=<optimized out>, clip=<optimized out>)
    at /home/george/dev/mozilla-central-2/gfx/cairo/cairo/src/cairo-image-surface.c:2307
#6  0x00007fffef68b650 in _clip_and_composite_polygon (
    dst=<error reading variable: DWARF-2 expression error: DW_OP_reg operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>, 
    op=<optimized out>, src=<optimized out>, polygon=<optimized out>, fill_rule=<optimized out>, antialias=<optimized out>, extents=<optimized out>, clip=<optimized out>)
    at /home/george/dev/mozilla-central-2/gfx/cairo/cairo/src/cairo-image-surface.c:3554
#7  0x00007fffef67f26f in _cairo_image_surface_fill (abstract_surface=<optimized out>, op=<optimized out>, source=<optimized out>, path=<optimized out>, 
    fill_rule=<optimized out>, tolerance=<optimized out>, antialias=<optimized out>, clip=<optimized out>)
    at /home/george/dev/mozilla-central-2/gfx/cairo/cairo/src/cairo-image-surface.c:3758

The line in question in pixman-sse2.c is:

  unit_y = src_image->common.transform->matrix[1][1];
Comment 5 George Wright (:gw280) (:gwright) 2012-12-18 00:43:53 PST
Yep, definitely looks like a legitimate buffer overflow to me. The previous line information was incorrect, but in a debug build I see that we're passing crazy width/height values into pixman, which looks like we're integer overflowing or similar. That int is then being used as an index to an array.
Comment 6 George Wright (:gw280) (:gwright) 2012-12-19 15:14:19 PST
Created attachment 694097 [details] [diff] [review]
Fix integer overflow

Found the expression that overflows, and it seems upstream fixed this already.
Comment 7 George Wright (:gw280) (:gwright) 2012-12-19 15:15:27 PST
Created attachment 694098 [details] [diff] [review]
Fix integer overflow

Erm, let's actually upload the correct patch this time...
Comment 8 David Bolter [:davidb] 2012-12-20 07:58:49 PST
Solid. What versions of FF are affected?
Comment 9 George Wright (:gw280) (:gwright) 2012-12-20 11:03:19 PST
Comment on attachment 694098 [details] [diff] [review]
Fix integer overflow

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not easily.

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

No.

Which older supported branches are affected by this flaw?

All.

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

N/A

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

This patch should cleanly apply to all current branches.

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

Very unlikely. No explicit testing required.
Comment 10 Al Billings [:abillings] 2012-12-20 11:34:09 PST
Cc'ing release management so we can talk about when we'd want to take this and give sec-approval. We're meeting today to go over Branch issues.
Comment 11 Alex Keybl [:akeybl] 2012-12-20 16:39:07 PST
Once Al gives sec-approval, please nominate for uplift to Aurora/Beta/ESR10/ESR17, making sure to land no later than 12/26.
Comment 12 Al Billings [:abillings] 2012-12-20 16:39:30 PST
Comment on attachment 694098 [details] [diff] [review]
Fix integer overflow

sec-approval+ on trunk.
Comment 13 Benoit Jacob [:bjacob] (mostly away) 2012-12-20 17:11:47 PST
This file seems to have other unchecked integer arithmetic that could overflow too, e.g. line 1056:

  src1 = src_first_line + src_stride * y1;

The most productive thing to do here would probably be to build this in IOC.

I assume that using CheckedInt here wasn't considered because pixman must remain free of dependency on MFBT.
Comment 14 George Wright (:gw280) (:gwright) 2012-12-27 09:35:33 PST
Landed on m-c:

https://hg.mozilla.org/mozilla-central/rev/7457db98e2e3
Comment 15 George Wright (:gw280) (:gwright) 2012-12-27 09:41:40 PST
Comment on attachment 694098 [details] [diff] [review]
Fix integer overflow

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec-critical
Fix Landed on Version: 20 (m-c trunk)
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec-critical
Fix Landed on Version: 20 (m-c trunk)
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: potential for critical security exploit
Testing completed (on m-c, etc.): In m-c now.
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-12-27 16:47:22 PST
Is there any reason this isn't marked as FIXED if it's on m-c?
Comment 17 George Wright (:gw280) (:gwright) 2012-12-27 17:06:32 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/59639b84eb14
https://hg.mozilla.org/releases/mozilla-aurora/rev/bdca00e901f1
https://hg.mozilla.org/releases/mozilla-esr17/rev/08c18065b666

The patch doesn't apply cleanly to esr10. I'm currently investigating whether esr10 is actually affected or not.
Comment 18 George Wright (:gw280) (:gwright) 2012-12-27 17:19:36 PST
esr10 doesn't appear to be affected (or at least, I can't reproduce it locally with esr10) so closing it for that.
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-12-28 09:49:07 PST
https://hg.mozilla.org/releases/mozilla-b2g18/rev/59639b84eb14
Comment 20 juan becerra [:juanb] 2013-01-02 13:33:28 PST
George, is there a way to verify this? When I try earlier Firefox beta versions with the testcase here, I am prompted by the OS to close after a while. With the latest beta where the fix landed, I have to restart my VM in order to get out of it. So the problem might be fixed, but the symptom is a little worse with the fix.

Used Fx18b7 candidates on a Mint linux VM.
Comment 21 George Wright (:gw280) (:gwright) 2013-01-02 14:04:05 PST
I can reproduce locally. Will investigate. This may require a new bug.
Comment 22 George Wright (:gw280) (:gwright) 2013-01-02 15:35:05 PST
It seems to only crash in that way when running inside a virtual machine (VMware Fusion 5.0.1 in my case). What VM software are you running your Mint instance in?
Comment 23 juan becerra [:juanb] 2013-01-02 15:49:32 PST
I was running the VM on Fusion 4.1.3.

On hardware I don't see the problem in comment #20. With earlier versions I get a freeze when opening the test case. With Fx18b7 the browser remains responsive.

George, if you don't have any further concerns after finding out this only happens on VMWare, I'll go ahead and mark this as verified.
Comment 24 George Wright (:gw280) (:gwright) 2013-01-02 15:53:50 PST
Can you try setting the pref gfx.xrender.enabled to false and see if that fixes the crash in VMware?
Comment 25 George Wright (:gw280) (:gwright) 2013-01-02 15:55:15 PST
(By the way, I think this is a very uncommon use case so unless we can confirm the latest lockup is a buffer overflow or similar, I don't see this as a critical bug for beta/aurora/etc).
Comment 26 juan becerra [:juanb] 2013-01-02 16:24:57 PST
Setting the preference mentioned in comment #24 the problem goes away; the browser remains responsive.
Comment 27 George Wright (:gw280) (:gwright) 2013-01-02 16:41:51 PST
Looks like a driver bug in VMware then. I'll loop back with them upstream and discuss with others in graphics what to do about it.

Note You need to log in before you can comment on or make changes to this bug.