1468131 - UBSan: pointer index expression overflowed [@ GetTrimmableWhitespaceCount]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found with commit 422090:874dedd55599

layout/generic/nsTextFrame.cpp:880:38: runtime error: pointer index expression with base 0x000000000000 overflowed to 0xffffffffffffffff
    #0 0x7fb033996b5c in GetTrimmableWhitespaceCount(nsTextFragment const*, int, int, int) layout/generic/nsTextFrame.cpp:880:38
    #1 0x7fb033996917 in nsTextFrame::GetTrimmedOffsets(nsTextFragment const*, bool, bool) const layout/generic/nsTextFrame.cpp:2966:7
    #2 0x7fb0339986ff in PropertyProvider::InitializeForDisplay(bool) layout/generic/nsTextFrame.cpp:3689:13
    #3 0x7fb03399c1f7 in nsTextFrame::PaintText(nsTextFrame::PaintTextParams const&, nsCharClipDisplayItem const&, float) layout/generic/nsTextFrame.cpp:6958:12
    #4 0x7fb03399be75 in nsDisplayText::RenderToContext(gfxContext*, nsDisplayListBuilder*, bool) layout/generic/nsTextFrame.cpp:5181:6
    #5 0x7fb03399bb25 in nsDisplayText::Paint(nsDisplayListBuilder*, gfxContext*) layout/generic/nsTextFrame.cpp:5090:3
    #6 0x7fb033b21984 in mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::AssignedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float) layout/painting/FrameLayerBuilder.cpp:6438:15
    #7 0x7fb033b2224f 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*) layout/painting/FrameLayerBuilder.cpp:6595:19
    #8 0x7fb03167e9cd in mozilla::layers::ClientPaintedLayer::PaintThebes(nsTArray<mozilla::layers::ReadbackProcessor::Update>*) gfx/layers/client/ClientPaintedLayer.cpp:158:5
    #9 0x7fb03167f59f in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) gfx/layers/client/ClientPaintedLayer.cpp:314:3
    #10 0x7fb031691b06 in mozilla::layers::ClientContainerLayer::RenderLayer() gfx/layers/client/ClientContainerLayer.h:58:29
    #11 0x7fb03167c866 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) gfx/layers/client/ClientLayerManager.cpp:372:13
    #12 0x7fb03167cc40 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) gfx/layers/client/ClientLayerManager.cpp:430:3
    #13 0x7fb033b444d3 in nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) layout/painting/nsDisplayList.cpp:2798:19
    #14 0x7fb033833fbf in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) layout/base/nsLayoutUtils.cpp:3843:12
    #15 0x7fb0337d48c6 in mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) layout/base/PresShell.cpp:6314:5
    #16 0x7fb033557106 in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) view/nsViewManager.cpp:480:19
    #17 0x7fb033556d5a in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) view/nsViewManager.cpp:412:33
    #18 0x7fb033557c6a in nsViewManager::ProcessPendingUpdates() view/nsViewManager.cpp:1102:5
    #19 0x7fb0337a4e0e in nsRefreshDriver::Tick(long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:2039:11
    #20 0x7fb0337aabe1 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) layout/base/nsRefreshDriver.cpp:301:7
    #21 0x7fb0337aaac8 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:320:5
    #22 0x7fb0337ace4b in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:760:5
    #23 0x7fb0337ac4ad in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:673:35
    #24 0x7fb0337ac125 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:574:9
    #25 0x7fb033ad5aee in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) layout/ipc/VsyncChild.cpp:68:16
    #26 0x7fb030df1e70 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) objdir-ff-ubsan/ipc/ipdl/PVsyncChild.cpp:167:20
    #27 0x7fb030a5d84c in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp:2134:25
    #28 0x7fb030a5c85c in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) ipc/glue/MessageChannel.cpp:2064:17
    #29 0x7fb030a5d2b0 in mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1943:15
    #30 0x7fb03025c472 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1088:14
    #31 0x7fb03027af5e in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:519:10
    #32 0x7fb030a6037f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:97:21
    #33 0x7fb0309c9a89 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:298:3
    #34 0x7fb033597fff in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:157:27
    #35 0x7fb03531a993 in XRE_RunAppShell() toolkit/xre/nsEmbedFunctions.cpp:896:22
    #36 0x7fb030a60a98 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:269:9
    #37 0x7fb0309c9a89 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:298:3
    #38 0x7fb03531a552 in XRE_InitChildProcess(int, char**, XREChildData const*) toolkit/xre/nsEmbedFunctions.cpp:722:34
    #39 0x430a7a in content_process_main(mozilla::Bootstrap*, int, char**) browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #40 0x430b40 in main browser/app/nsBrowserApp.cpp:287:18
    #41 0x7fb052e7c1c0 in __libc_start_main /build/glibc-itYbWN/glibc-2.26/csu/../csu/libc-start.c:308
    #42 0x4092e9 in _start (firefox+0x4092e9)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

It feels like something is really going wrong given the error description...

Comment 3

[(Away)]

    const char* str = aFrag->Get1b() + aStartOffset;
    for (; count < aLength; ++count) {
      if (!IsTrimmableSpace(*str)) break;
      str += aDirection;
    }

My debugger says:
aStartOffset = 0n-1
aLength = 0n0
aDirection = 0n-1
and aFrag's string pointer is null.

So we construct this invalid pointer by doing nullptr + (-1) but we never read from it because the for loop's condition is never satisfied.

What is the preferred technique for dealing with this? Should we add a aLength > 0 check to appease the checker? Should we consider it a false positive? Should the caller be modified not to call this function if length is 0?

Comment 4

[(Away)]

Attachment: Avoid a ubsan complaint in GetTrimmableWhitespaceCount

ubsan was complaining about the expression

    const char* str = aFrag->Get1b() + aStartOffset;

when aFrag->Get1b() == nullptr and aStartOffset == -1, because the addition generates an invalid pointer.

Due to other logic in the function, we would never dereference that pointer, so it was reasonably harmless, but this patch silences the complaint.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f934e73d44c
Avoid a ubsan complaint in GetTrimmableWhitespaceCount r=emilio

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4f934e73d44c