1801248 - gecko/gfx/thebes/gfxTextRun.cpp:410:31: runtime error: pointer index expression overflowed [@ gfxTextRun::GetAdjustedSpacingArray]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing (--enable-undefined-sanitizer --enable-fuzzing)

This was found while testing 32-bit fuzzing on Linux. The attached test case requires a 32-bit build with UBSan (specificly the pointer-overflow check).

/builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:410:31: runtime error: pointer index expression with base 0xffca46bc overflowed to 0x00003b3c
    #0 0xdb3e9e04 in gfxTextRun::GetAdjustedSpacingArray(gfxTextRun::Range, gfxTextRun::PropertyProvider*, gfxTextRun::Range, nsTArray<gfxFont::Spacing>*) const /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:410:31
    #1 0xdb3f0d9a in gfxTextRun::AccumulateMetricsForRun(gfxFont*, gfxTextRun::Range, gfxFont::BoundingBoxType, mozilla::gfx::DrawTarget*, gfxTextRun::PropertyProvider*, gfxTextRun::Range, mozilla::gfx::ShapedTextFlags, gfxFont::RunMetrics*) const /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:774:7
    #2 0xdb3ef06e in gfxTextRun::MeasureText(gfxTextRun::Range, gfxFont::BoundingBoxType, mozilla::gfx::DrawTarget*, gfxTextRun::PropertyProvider*) const /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:842:5
    #3 0xdb3f4ea9 in gfxTextRun::BreakAndMeasureText(unsigned int, unsigned int, bool, double, gfxTextRun::PropertyProvider*, gfxTextRun::SuppressBreak, double*, bool, gfxFont::RunMetrics*, gfxFont::BoundingBoxType, mozilla::gfx::DrawTarget*, bool*, unsigned int*, bool, bool, gfxBreakPriority*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:1195:11
    #4 0xe4172e6a in nsTextFrame::ReflowText(nsLineLayout&, int, mozilla::gfx::DrawTarget*, mozilla::ReflowOutput&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:9764:44
    #5 0xe416dd6e in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:873:40
    #6 0xe3ebd47e in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4718:15
    #7 0xe3ebb5f0 in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4520:5
    #8 0xe3eb210d in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4394:9
    #9 0xe3ea966e in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3380:5
    #10 0xe3e9cde9 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2894:9
    #11 0xe3e9374f in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1470:3
    #12 0xe3ee158c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1029:14
    #13 0xe3edfa04 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsCanvasFrame.cpp:755:7
    #14 0xe3ee158c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1029:14
    #15 0xe3f91184 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput&, bool, bool, mozilla::ReflowOutput*) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:841:3
    #16 0xe3f93d3d in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput&, mozilla::ReflowOutput const&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:977:3
    #17 0xe3f9bdfa in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:1404:3
    #18 0xe3e7bf59 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1069:14
    #19 0xe3e7b270 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:383:7
    #20 0xe3c188a6 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9662:11
    #21 0xe3c66805 in mozilla::PresShell::ProcessReflowCommands(bool) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9834:24
    #22 0xe3c2fe22 in DoFlushLayout /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9904:10
    #23 0xe3c2fe22 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4413:11
    #24 0xdba820a4 in mozilla::PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1474:5
    #25 0xe3b892cf in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2614:20
    #26 0xe3ba087c in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:375:13
    #27 0xe3ba087c in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver>>&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:353:7
    #28 0xe3ba026f in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:369:5
    #29 0xe3b9fbd2 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:913:5
    #30 0xe3b9dc64 in mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:827:5
    #31 0xe3b9c10c in mozilla::VsyncRefreshDriverTimer::NotifyVsyncOnMainThread(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:748:5
    #32 0xe3b9b6a3 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:594:14
    #33 0xe3b9b125 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:551:9
    #34 0xe2212a10 in mozilla::dom::VsyncMainChild::RecvNotify(mozilla::VsyncEvent const&, float const&) /builds/worker/checkouts/gecko/dom/ipc/VsyncMainChild.cpp:68:15
    #35 0xe276ebfc in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:220:78
    #36 0xe255d970 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentChild.cpp:8699:32
    #37 0xd9c13fc7 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1756:25
    #38 0xd9c104d6 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1681:9
    #39 0xd9c113e1 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1481:3
    #40 0xd9c12a25 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1579:14
    #41 0xd7e88882 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:538:16
    #42 0xd7e7b802 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:851:26
    #43 0xd7e777be in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:683:15
    #44 0xd7e781a6 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:461:36
    #45 0xd7e91410 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:187:37
    #46 0xd7e91410 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_2>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:546:5
    #47 0xd7ec2d14 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1204:16
    #48 0xd7ed224a in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:474:10
    #49 0xd9c1d662 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
    #50 0xd9c1f662 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:268:30
    #51 0xd9a28877 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
    #52 0xd9a28877 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
    #53 0xd9a28877 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
    #54 0xe340fc5f in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:150:27
    #55 0xe9462d37 in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:884:20
    #56 0xd9a28877 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
    #57 0xd9a28877 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
    #58 0xd9a28877 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
    #59 0xe946172c in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:743:34
    #60 0xe94794d1 in mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/Bootstrap.cpp:67:12
    #61 0x5679f77b in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #62 0x5679fced in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:359:18
    #63 0xf7937518  (/lib/i386-linux-gnu/libc.so.6+0x21518) (BuildId: 0494f075afbcfa9004eaaedccbea53807b7bf669)
    #64 0xf79375f2 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x215f2) (BuildId: 0494f075afbcfa9004eaaedccbea53807b7bf669)
    #65 0x566dd500 in _start (/home/user/workspace/browsers/linux32-try-20221116212757-fuzzing-asan-opt/firefox-bin+0xf4500) (BuildId: c4b3935adcb862a4a25d91b2449386e39daddf3b)

SUMMARY: UndefinedBehaviorSanitizer: pointer-overflow /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:410:31 in 

Comment 1

[Jonathan Kew [:jfkthame]]

The stack here doesn't make any sense to me (where do all those table-related frames come from?), but from inspecting the code at the location of the error, the only idea I currently have is to re-write the expression on line 410 as:

diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -407,7 +407,7 @@ bool gfxTextRun::GetAdjustedSpacingArray
   memset(aSpacing->Elements(), 0, sizeof(gfxFont::Spacing) * spacingOffset);
   GetAdjustedSpacing(this, aSpacingRange, aProvider,
                      aSpacing->Elements() + spacingOffset);
-  memset(aSpacing->Elements() + aSpacingRange.end - aRange.start, 0,
+  memset(aSpacing->Elements() + spacingOffset + aSpacingRange.Length(), 0,
          sizeof(gfxFont::Spacing) * (aRange.end - aSpacingRange.end));
   return true;
 }

But I don't have a suitable build on hand to reproduce... Tyson, can you check if this makes any difference? Thanks!

Comment 2

[Tyson Smith [:tsmith]]

(In reply to Jonathan Kew [:jfkthame] from comment #1)

The stack here doesn't make any sense to me (where do all those table-related frames come from?),

Sorry that's my fault, that stack is from one of the other less reliable test cases. I've replaced the stack with one generated from the attached test case.

But I don't have a suitable build on hand to reproduce... Tyson, can you check if this makes any difference? Thanks!

Yep of course.

Comment 3

[Tyson Smith [:tsmith]]

(In reply to Jonathan Kew [:jfkthame] from comment #1)

Tyson, can you check if this makes any difference? Thanks!

I am not able to reproduce the issue with the patch applied. Thanks :)

Comment 4

[Jonathan Kew [:jfkthame]]

I think that provided the pointer arithmetic here is implemented as unsigned 32-bit arithmetic, with wraparound on over-/underflow, the end result would be correct: adding aSpacingRange.end to the pointer aSpacing->Element() may overflow, but then when we subtract aRange.start that'll underflow and wrap back to the expected value. So in practice nothing bad happens. But pointer arithmetic overflow is not actually defined by the standard (AFAIK), so we shouldn't be relying on this.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1801248 - Fix up pointer arithmetic. r=#gfx-reviewers

Comment 6

[Jonathan Kew [:jfkthame]]

Based on the understanding in comment 4, I think it's unlikely there's anything exploitable here; although the overflow behavior is undefined per spec, in practice it'll still give the intended result. As such, this could be considered (at most) sec-low.... Tyson, does that seem OK to you?

Comment 7

[Tyson Smith [:tsmith]]

I agree. Thank you for your analysis and patch.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2651f65b7d4
Fix up pointer arithmetic. r=gfx-reviewers,lsalzman

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/a2651f65b7d4