Tested on: OS:Ubuntu 12.04 Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1382322354/ ASAN-report: ==2874==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x619000628de1 at pc 0x7fe55ce705ff bp 0x7fff2819a2f0 sp 0x7fff2819a2e8 READ of size 1 at 0x619000628de1 thread T0 #0 0x7fe55ce705fe in nsSVGTextFrame2::ResolvePositions(nsIContent*, unsigned int, bool, bool&, nsTArray<gfxPoint>&) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:4294:0 #1 0x7fe55ce7030b in nsSVGTextFrame2::ResolvePositions(nsIContent*, unsigned int, bool, bool&, nsTArray<gfxPoint>&) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:4363:0 #2 0x7fe55ce716d9 in nsSVGTextFrame2::ResolvePositions(nsTArray<gfxPoint>&, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:4408:0 #3 0x7fe55ce76523 in nsSVGTextFrame2::DoGlyphPositioning() /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:4848:0 #4 0x7fe55ce6984a in UpdateGlyphPositioning /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:5019:0 #5 0x7fe55ce6984a in nsSVGTextFrame2::GetBBoxContribution(gfxMatrix const&, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:3748:0 #6 0x7fe55ce6a70c in non-virtual thunk to nsSVGTextFrame2::GetBBoxContribution(gfxMatrix const&, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:3761:0 #7 0x7fe55ce8c002 in nsSVGUtils::GetBBox(nsIFrame*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGUtils.cpp:1244:0 #8 0x7fe55b274505 in GetFrameBoundsForTransform /builds/slave/m-cen-l64-asan-000000000000000/build/layout/base/nsDisplayList.cpp:3832:0 #9 0x7fe55b274505 in nsDisplayTransform::GetDeltaToMozTransformOrigin(nsIFrame const*, float, nsRect const*) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/base/nsDisplayList.cpp:3923:0 #10 0x7fe55b27593f in nsDisplayTransform::FrameTransformProperties::FrameTransformProperties(nsIFrame const*, float, nsRect const*) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/base/nsDisplayList.cpp:4037:0 . . .

There is also some weird asan outputs, later on on the trace, so I add also the full trace here. Weird ASAN-output: ==2874==AddressSanitizer CHECK failed: /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:228 "((id)) != (0)" (0x0, 0x0) #0 0x44bbf4 in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) _asan_rtl_:0 #1 0x450e21 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:60:0 #2 0x423122 in GetStackTraceFromId /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:228:0 #3 0x423122 in __asan::AsanChunkView::GetAllocStack(__sanitizer::StackTrace*) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:238:0 #4 0x448ce6 in __asan::DescribeHeapAddress(unsigned long, unsigned long) _asan_rtl_:0 #5 0x449dd4 in __asan_report_error _asan_rtl_:0 #6 0x44ae46 in __asan_report_load1 _asan_rtl_:0 #7 0x7fe55ce705fe in nsSVGTextFrame2::ResolvePositions(nsIContent*, unsigned int, bool, bool&, nsTArray<gfxPoint>&) /builds/slave/m-cen-l64-asan-000000000000000/build/layout/svg/nsSVGTextFrame2.cpp:4294:0 . . .

I can take a look at this next week, once I'm back at my machine with asan builds available.

We're calling in to nsSVGTextFrame2::GetBBoxContribution without its anonymous block child having been reflowed. This is happening now due to bug 923193 needing the bounding box of the frame when computing the box for transform-origin.

And scheduling of the PresShell::UpdateImageVisibility call, which is what builds display lists and ultimately causes GetBBoxContribution to be called, is racing against the scheduling of the nsSVGTextFrame2::ReflowSVG.

I think also that our early exit check at the top of nsSVGTextFrame2::GetBBoxContribution, which should have caught this case and returned an empty bounding box, should be looking at the dirtiness of the nsSVGTextFrame2, not its anonymous block child. The earlier nsSVGUtils::ScheduleReflowSVG call just sets dirty bits on the nsSVGTextFrame2.

I'm guessing the consequences of returning an incorrect value to nsDisplayTransform::GetFrameBoundsForTransform doesn't matter for PresShell::UpdateImageVisibility's purpose. But I also don't know whether it is bad or not that display lists are getting constructed for the dirty nsSVGTextFrame2.

Comment on attachment 823793 [details] [diff] [review] patch Review of attachment 823793 [details] [diff] [review]: ----------------------------------------------------------------- Building display lists for dirty trees needs to be supported. It doesn't necessarily need to produce the correct rendering, but it should be correct for the parts of the tree that aren't dirty.

Is this a firefox 27 regression ? Can we also get a security rating here to consider for esr.

Backed out (realised I forgot to request sec-approval): https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a356016ca4

Isn't the cat out the bag already once it hits the first time?

Comment on attachment 823793 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Pretty hard. The code change is small and doesn't show what you would need to do to exploit the bug. You'd need to know how nsSVGTextFrame2 works overall. 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? 27 If not all supported branches, which bug introduced the flaw? 923193 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The patch applies cleanly to mozilla-aurora. How likely is this patch to cause regressions; how much testing does it need? Fairly unlikely. IMO the manual testing I've done plus a green try run are sufficient.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11) > Isn't the cat out the bag already once it hits the first time? I guess so. Sorry about that... :(

Although going from patch to exploit would be quite hard, IMO.

In terms of severity of this bug, it lets you write right off the end of nsSVGTextFrame2::mPositions, which is on the heap, and the offset into mPositions that ResolvePositions writes to is determined by the length of the content under the <text> element.

(In reply to Cameron McCormack (:heycam) from comment #10) > Backed out (realised I forgot to request sec-approval): > https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a356016ca4 Merge of backout: https://hg.mozilla.org/mozilla-central/rev/f532e4f26307

Comment on attachment 823793 [details] [diff] [review] patch Well, since we (sort of) pwned ourselves by already checking it in once, there isn't much debate now. sec-approval+ for trunk. I also gave it Aurora approval since that is the other place affected.

https://hg.mozilla.org/integration/mozilla-inbound/rev/238b3c81f42c https://hg.mozilla.org/releases/mozilla-aurora/rev/27d0165a1097

Backed out along with other stuff related to bug 923193. https://hg.mozilla.org/integration/mozilla-inbound/rev/4a5c39f4cf22 I'll fold this back in when we reland bug 923193.

Merge of backout. https://hg.mozilla.org/mozilla-central/rev/3fb151446ec5 Going to re-resolve this based on comment 22 saying that this will be rolled into the re-landing of bug 923193.

Rather: https://hg.mozilla.org/mozilla-central/rev/4a5c39f4cf22

Confirmed crash on ASan FF27, 2013-09-25. Verified fixed on ASan FF27, 2013-11-20. Verified fixed on ASan FF28, 2013-11-22.