1420492 - UBSan: division by zero in [@ nsSVGArcConverter::nsSVGArcConverter]

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

This was found with a Firefox build built with -fsanitize=float-divide-by-zero,integer-divide-by-zero

/mozilla-central/dom/svg/nsSVGPathDataParser.cpp:453:36: runtime error: division by zero
    #0 nsSVGArcConverter::nsSVGArcConverter(mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, double, bool, bool) /mozilla-central/dom/svg/nsSVGPathDataParser.cpp:453:36
    #1 TraverseArcRel(float const*, mozilla::SVGPathTraversalState&) /mozilla-central/dom/svg/SVGPathSegUtils.cpp:396:23
    #2 mozilla::SVGPathData::GetDistancesFromOriginToEndsOfVisibleSegments(FallibleTArray<double>*) const /mozilla-central/dom/svg/SVGPathData.cpp:162:5
    #3 mozilla::SVGMotionSMILAnimationFunction::RebuildPathAndVerticesFromPathAttr() /mozilla-central/dom/svg/SVGMotionSMILAnimationFunction.cpp:257:18
    #4 mozilla::SVGMotionSMILAnimationFunction::RebuildPathAndVertices(nsIContent const*) /mozilla-central/dom/svg/SVGMotionSMILAnimationFunction.cpp:283:5
    #5 mozilla::SVGMotionSMILAnimationFunction::GetValues(nsISMILAttr const&, FallibleTArray<nsSMILValue>&) /mozilla-central/dom/svg/SVGMotionSMILAnimationFunction.cpp:324:5
    #6 nsSMILAnimationFunction::ComposeResult(nsISMILAttr const&, nsSMILValue&) /mozilla-central/dom/smil/nsSMILAnimationFunction.cpp:216:17
    #7 nsSMILCompositor::ComposeAttribute(bool&) /mozilla-central/dom/smil/nsSMILCompositor.cpp:106:29
    #8 nsSMILAnimationController::DoSample(bool) /mozilla-central/dom/smil/nsSMILAnimationController.cpp:455:17
    #9 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /mozilla-central/layout/base/PresShell.cpp:4203:46
    #10 FlushPendingNotifications /mozilla-central/objdir-ff-asan-O2/dist/include/nsIPresShell.h:571:5
    #11 nsDocument::FlushPendingNotifications(mozilla::FlushType, mozilla::FlushTarget) /mozilla-central/dom/base/nsDocument.cpp:8559
    #12 nsDocLoader::DocLoaderIsEmpty(bool) /mozilla-central/uriloader/base/nsDocLoader.cpp:704:14
    #13 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /mozilla-central/uriloader/base/nsDocLoader.cpp:633:5
    #14 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /mozilla-central/uriloader/base/nsDocLoader.cpp
    #15 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /mozilla-central/netwerk/base/nsLoadGroup.cpp:629:28
    #16 nsDocument::DoUnblockOnload() /mozilla-central/dom/base/nsDocument.cpp:9388:18
    #17 nsDocument::UnblockOnload(bool) /mozilla-central/dom/base/nsDocument.cpp:9310:9
    #18 nsDocument::DispatchContentLoadedEvents() /mozilla-central/dom/base/nsDocument.cpp:5677:3
    #19 applyImpl<nsDocument, void (nsDocument::*)()> /mozilla-central/objdir-ff-asan-O2/dist/include/nsThreadUtils.h:1142:12
    #20 apply<nsDocument, void (nsDocument::*)()> /mozilla-central/objdir-ff-asan-O2/dist/include/nsThreadUtils.h:1148
    #21 mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /mozilla-central/objdir-ff-asan-O2/dist/include/nsThreadUtils.h:1192
    #22 mozilla::SchedulerGroup::Runnable::Run() /mozilla-central/xpcom/threads/SchedulerGroup.cpp:396:25
    #23 nsThread::ProcessNextEvent(bool, bool*) /mozilla-central/xpcom/threads/nsThread.cpp:1037:14
    #24 NS_ProcessNextEvent(nsIThread*, bool) /mozilla-central/xpcom/threads/nsThreadUtils.cpp:513:10
    #25 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /mozilla-central/ipc/glue/MessagePump.cpp:97:21
    #26 RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319:3
    #27 MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #28 nsBaseAppShell::Run() /mozilla-central/widget/nsBaseAppShell.cpp:159:27
    #29 XRE_RunAppShell() /mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:877:22
    #30 RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319:3
    #31 MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #32 XRE_InitChildProcess(int, char**, XREChildData const*) /mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:703:34
    #33 content_process_main /mozilla-central/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #34 main /mozilla-central/browser/app/nsBrowserApp.cpp:280
    #35 __libc_start_main /build/glibc-CxtIbX/glibc-2.26/csu/../csu/libc-start.c:308
    #36 _start (firefox+0x41efc9)

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch with test

nsSVGArcConverter::nsSVGArcConverter is not designed to deal with zero radius arcs it expects the caller to deal with them as they are treated as straight lines per https://www.w3.org/TR/SVG/implnote.html#ArcOutOfRangeParameters

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 8931834 [details] [diff] [review]
patch with test

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

Thanks!

r=me, though ideally it'd be great to add a testcase that directly tests the measured (straight-line) length, via e.g. some myPathElem.getTotalLength() call...  (I'm assuming that getTotalLength() call would end up exercising this same code.)

Comment 3

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c572ce09fdaeca703602e9b7de9ea647ba4d02bc

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/990365a457ca
avoid division by zero when constructing an nsSVGArcConverter r=dholbert

Comment 5

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/990365a457ca