802638 - Heap-use-after-free/heap-buffer-overflow in nsRegion::nsRectFast::Intersects, with SVG markers

Status: RESOLVED DUPLICATE of bug 795734

(Core :: SVG, defect)

Description

[Arthur Gerkis]

Attachment: test-case triggering the crash (*.zip) - open file run.html.

ASan detects two types of issues while running attached test-case: use-after-free or heap-buffer-overflow. It may also crash  while trying to access address like 0x003c000000b8 (rarely in case). Sorry for not badly reduced test-case - depending on minimized version, ASan may report one of those issues.

Test-case tested on http://hg.mozilla.org/mozilla-central/rev/942ed5747b63. Crashes also on Windows version 19.0a1 (2012-10-17), but for Windows stack looks bit different.

Comment 1

[Arthur Gerkis]

Attachment: ASan log for both crash types

Comment 2

[Daniel Holbert [:dholbert]]

In my debug build, I hit this NS_ABORT_IF_FALSE before crashing, when the title of the tab reaches "47":

 ###!!! ABORT: Passed bad frame!: 'aFrame->IsFrameOfType(nsIFrame::eSVG) && !(aFrame->GetStateBits() & NS_STATE_IS_OUTER_SVG)', file /mozilla/layout/svg/nsSVGUtils.cpp, line 385

Comment 3

[Daniel Holbert [:dholbert]]

(er s/before crashing/and abort instead of crashing/)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: backtrace of ABORT_IF_FALSE failure

Comment 5

[Daniel Holbert [:dholbert]]

The frame failing the check there is a nsSVGOuterSVGFrame, which makes me wonder if this is a dupe of bug 798010.  (same frame, failing the same assertion, per bug 798010 comment 5 - 6).

Marking as dependency now, to establish a relationship. When that bug is fixed, we can see if it fixes this bug as well.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> The frame failing the check there is a nsSVGOuterSVGFrame, which makes me
> wonder if this is a dupe of bug 798010.

More confirmation of this suspicion: up a few levels from the abort, we're in  nsSVGMarkerProperty::DoUpdate() -- and bug 798010 involves SVG markers.  So: likely a dupe, at least for that first abort.

Comment 7

[Jonathan Watt [:jwatt]]

Almost certainly fixed by bug 807213, but I won't have an updated ASAN build to check until tomorrow.

Comment 8

[Arthur Gerkis]

I guess patch has not landed in mozilla central yet, but just fyi - my test-case diesn't crash with uaf or hbo anymore - it is just a crash on unknown address. Build 2cb6d26fcfe7.

Comment 9

[Jonathan Watt [:jwatt]]

(In reply to Arthur Gerkis from comment #8)
> I guess patch has not landed in mozilla central yet, but just fyi - my
> test-case diesn't crash with uaf or hbo anymore - it is just a crash on
> unknown address. Build 2cb6d26fcfe7.

I tested an ASAN build created after the fix in bug 807213 hand landed and it reported no issues. I've not tested a build prior to that, so maybe the issue had already been fixed though.

At any rate the fix in bug 807213 has now landed for 19, 18 and 17, and I couldn't reproduce any issue on the ASAN m-c build I created, so closing.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Did this affect Firefox 16 or ESR10? When did it start as an issue?

Comment 11

[Matt Wobensmith [:mwobensmith][:matt:]]

Does not affect ESR10. Ran test case against 10.0.10 as well as 10.0.1 (for fun) and no crash.

Comment 12

[u279076]

I believe comment 11 addresses the qawanted request. Please re-add if there's something more we can do.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Was this a dupe of bug 798010 or not?

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Watt [:jwatt] from comment #9)
> I tested an ASAN build created after the fix in bug 807213 hand landed and
> it reported no issues. I've not tested a build prior to that, so maybe the
> issue had already been fixed though.

I tested a (non-ASAN) debug build, built from revision 734c42e76813 (bug 807213's fix), and it loads the testcase just fine -- no abort, no issues. If I revert that cset (going back in time by 1 commit), then the testcase triggers the abort that I mentioned in comment 2.

So -- bug 807213 is definitely what fixed that abort, and it's almost certainly what fixed the ASAN issue as well.  Not sure if "dupe" is exactly right, since bug 807213 was more speculative & didn't have a testcase of its own (see bug 807213 comment 0), but I think it's definitely safe to say that this was fixed by that bug's patch.

Comment 15

[Daniel Holbert [:dholbert]]

Oh, I misread comment 13. As I was saying, this isn't exactly a dupe of bug 807213, but it is a dupe of bug 798010 and bug 795734. (marker-related security bugs fixed by bug 807213)

Marking as such.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Not writing an advisory for this because it was fixed by security bug 795734, which pre-dates this bug by a bit and is already getting an advisory.

Comment 17

[Jonathan Watt [:jwatt]]

Thanks, Daniel.