Closed Bug 783041 Opened 12 years ago Closed 12 years ago

out-of-bounds read when blurring

Categories

(Core :: Graphics, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla17
Tracking Status
firefox14 --- affected
firefox15 --- affected
firefox16 --- affected
firefox17 --- fixed
firefox-esr10 --- wontfix

People

(Reporter: miaubiz, Assigned: MatsPalmgren_bugz)

References

Details

(5 keywords, Whiteboard: [asan][adv-track-main17+])

Attachments

(9 files)

Attached file repro case
opening this html crashes asan:

<html>
  <head>
    <style>
      #el7 {
        font-size:.92em 
      } 
      #el0 { 
        height: 200px ! important; 
        margin: 0px; 
        display: table;
        font-size:.92em 
      }

      #el3 { 
        line-height: 0.5px; 
        text-shadow: 0px 5px 5px, 0px -20px 10px;
        display: table-row-group; 
        transform: translate3d(-3px, -300px, 0px); 
      }

      #el5 { 
        height:1em;
        display:block; 
      } 

      .c4 { 
        margin: 1em;
        padding: 0.5em;
      } 
    </style>
    <script>
      onload = function() {
        el7=document.createElement('iframe')
        el7.setAttribute('id', 'el7')
        document.body.appendChild(el7)

        el0=document.createElement('span')
        el0.setAttribute('id','el0')
        document.body.appendChild(el0)
        el0.appendChild(document.createTextNode('A'))

        el3=document.createElement('q')
        el3.setAttribute('id','el3')
        el0.appendChild(el3)

        el5=document.createElement('q')
        el5.setAttribute('id','el5')
        el3.appendChild(el5)

        el0.appendChild(document.createTextNode('A'))

        document.body.offsetTop
        el0.setAttribute('class', 'c4'); 
        el7.setAttribute('class', 'c4'); 
      }
    </script>
  </head>
  <body>
  </body>
</html>


like so:

==7766== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7fffc8308570 at pc 0x7ffff18196d7 bp 0x7fffffff22c0 sp 0x7fffffff22b8
READ of size 1 at 0x7fffc8308570 thread T0
    #0 0x7ffff18196d7 in mozilla::gfx::BoxBlurVertical(unsigned char*, unsigned char*, int, int, int, int, mozilla::gfx::IntRect const&) /builds/slave/try-lnx64/build/gfx/2d/Blur.cpp:148
0x7fffc8308570 is located 16 bytes to the left of 1-byte region [0x7fffc8308580,0x7fffc8308581)
allocated by thread T0 here:
    #0 0x424331 in __interceptor_malloc ??:0
    #1 0x7ffff1817c86 in mozilla::gfx::AlphaBoxBlur::Blur() /builds/slave/try-lnx64/build/gfx/2d/Blur.cpp:443

==80621== ERROR: AddressSanitizer heap-buffer-overflow on address 0x0001297b6e78 at pc 0x1080f96f7 bp 0x7fff5fbeaa60 sp 0x7fff5fbeaa58
READ of size 1 at 0x0001297b6e78 thread T0
    #0 0x1080f96f7 in _ZN7mozilla3gfxL15BoxBlurVerticalEPhS1_iiiiRKNS0_7IntRectE (in XUL) (Blur.cpp:148)
0x0001297b6e78 is located 8 bytes to the left of 1-byte region [0x0001297b6e80,0x0001297b6e81)
allocated by thread T0 here:
    #0 0x10000bd1d in (anonymous namespace)::mz_malloc(_malloc_zone_t*, unsigned long) (in firefox) + 45
    #1 0x7fff92d583c8 in __strtodg$UNIX2003 (in libsystem_c.dylib) + 945
    #2 0x7fff92d591a4 in __strtodg$UNIX2003 (in libsystem_c.dylib) + 4493
    #3 0x1080f7cb6 in mozilla::gfx::AlphaBoxBlur::Blur() (in XUL) (Blur.cpp:443)


I've seen anything from 4 to 84 bytes to the left depending on the exact values.
Attached file asan log linux
Attached file asan log osx
Attached file alternative repro
Attached file alternative repro
Attached file last one
I get a fatal assertion on the first and fourth testcase using
a Linux64 asan debug build:
Assertion failure: aRows > 0, at gfx/2d/Blur.cpp:119
Assignee: nobody → matspal
Severity: normal → critical
Component: Layout → Graphics
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [asan]
Attached patch part 1, fixSplinter Review
The problem is that we test IsEmpty() on 'rect' which is a gfx Rect
which use floating point members, and then later copying those
to 'mRect' which is IntRect, so a small rect.height makes IsEmpty()
false but mRect.height zero.  Later we depend on 'mData' being
NULL for an empty 'mRect' - the "aRows" in the assertion comes from
'mRect.height'.

Testing mRect.IsEmpty() instead should fix it.

I don't see anything that would do an illegal write operation
in this file (for either zero width/height) so the bug appears
non-exploitable, just an illegal read operation, which should
crash safely.

This patch applies also to Aurora and Beta.
The code is slightly different on esr10, but it looks like it's
also affected (gfxAlphaBoxBlur::Init in file gfx/thebes/gfxBlur.cpp).
It should be easy to make a similar fix for esr10.

https://tbpl.mozilla.org/?tree=Try&rev=aa94c6c8986e
Attachment #652243 - Flags: review?(roc)
'shadowIntRect' is redundant now - we can use mRect instead
which already is an IntRect.  s/IntersectRect/Intersect
since we don't use the return value of the former.
Attachment #652246 - Flags: review?(roc)
Blocks: 771842
if the adjacent memory is mapped readable, can the result of the bluring be used to leak contents of memory?
(In reply to miaubiz from comment #10)
> can the result of the bluring be
> used to leak contents of memory?

No, the read values are only stored in a local variable and
does not propagate any further when the bug occurs.

The code is written in such a way that the write operations
does not occur when aWidth or aRows is zero:
http://hg.mozilla.org/mozilla-central/file/50e4ff05741e/gfx/2d/Blur.cpp
miaubiz@gmail.com, I slapped the standard MPL2 onto your files:

<!-- This Source Code Form is subject to the terms of the Mozilla Public
   - License, v. 2.0. If a copy of the MPL was not distributed with this
   - file, You can obtain one at http://mozilla.org/MPL/2.0/.  -->

OK?
@mats: ok with me
Keywords: csec-dos
Blocks: 774207
Sec-other implies that this bug is not exploitable. Is that the case? https://wiki.mozilla.org/Security_Severity_Ratings
Yes, it's not exploitable AFAICT, see comment 7 and comment 11.
sec-other, non-exploitable, falls outside of esr criteria - wontfixing.
Keywords: verifyme
Since this is not exploitable there shouldn't be a problem landing the tests and unhiding the bug.
Should we flag this status-firefox14:wontfix and status-firefox15:wontfix at this point?
Whiteboard: [asan] → [asan][adv-track-main17+]
Alias: CVE-2012-5832
Alias: CVE-2012-5832
Group: core-security
Flags: sec-bounty-
Crash tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c955c4e5eaff
Flags: in-testsuite? → in-testsuite+
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: