Closed Bug 1573049 Opened 8 months ago Closed 7 months ago

Silence ubsan unsigned offset overflowed false-positive in [@ mozilla::gfx::Point3DTyped]

Categories

(Core :: Graphics, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: tsmith, Assigned: lsalzman)

References

(Blocks 2 open bugs)

Details

(Keywords: testcase)

Attachments

(2 files)

Attached file testcase.html

Found with m-c 20190810-c53f789ffabb

This was build with undefined behavior sanitizer checks enabled via mozconfig.
ac_add_options --enable-undefined-sanitizer="pointer-overflow"

src/gfx/2d/FilterNodeSoftware.cpp:2304:10: runtime error: addition of unsigned offset to 0x6300000006c2 overflowed to 0x630000000561
    #0 0x7f1640cd4cbd in mozilla::gfx::Point3DTyped<mozilla::gfx::UnknownUnits, float> mozilla::gfx::GenerateNormal<int>(unsigned char const*, int, unsigned char*, unsigned char*, int, int, float, int, int) src/gfx/2d/FilterNodeSoftware.cpp
    #1 0x7f1640ce2a98 in DoRender<int> src/gfx/2d/FilterNodeSoftware.cpp:3635:11
    #2 0x7f1640ce2a98 in mozilla::gfx::FilterNodeLightingSoftware<mozilla::gfx::(anonymous namespace)::DistantLightSoftware, mozilla::gfx::(anonymous namespace)::SpecularLightingSoftware>::Render(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/gfx/2d/FilterNodeSoftware.cpp:3546
    #3 0x7f1640c8b86c in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/gfx/2d/FilterNodeSoftware.cpp:638:20
    #4 0x7f1640c8f60a in mozilla::gfx::FilterNodeSoftware::GetInputDataSourceSurface(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::FilterNodeSoftware::FormatHint, mozilla::gfx::ConvolveMatrixEdgeMode, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) src/gfx/2d/FilterNodeSoftware.cpp:770:25
    #5 0x7f1640ca9b26 in mozilla::gfx::FilterNodeCropSoftware::Render(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/gfx/2d/FilterNodeSoftware.cpp:3166:10
    #6 0x7f1640c8b86c in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/gfx/2d/FilterNodeSoftware.cpp:638:20
    #7 0x7f1640c8aacd in mozilla::gfx::FilterNodeSoftware::Draw(mozilla::gfx::DrawTarget*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::DrawOptions const&) src/gfx/2d/FilterNodeSoftware.cpp:572:14
    #8 0x7f1640c5587b in mozilla::gfx::DrawFilterCommand::ExecuteOnDT(mozilla::gfx::DrawTarget*, mozilla::gfx::BaseMatrix<float> const*) const src/gfx/2d/DrawCommands.h:223:10
    #9 0x7f1640bea6b5 in mozilla::gfx::DrawTargetCaptureImpl::ReplayToDrawTarget(mozilla::gfx::DrawTarget*, mozilla::gfx::BaseMatrix<float> const&) src/gfx/2d/DrawTargetCapture.cpp:330:10
    #10 0x7f1640bea4e5 in mozilla::gfx::DrawTarget::DrawCapturedDT(mozilla::gfx::DrawTargetCapture*, mozilla::gfx::BaseMatrix<float> const&) src/gfx/2d/DrawTarget.cpp:168:9
    #11 0x7f1640fcae3c in mozilla::layers::PaintThread::AsyncPaintTask(mozilla::layers::CompositorBridgeChild*, mozilla::layers::PaintTask*) src/gfx/layers/PaintThread.cpp:206:13
    #12 0x7f164101e0e3 in operator() src/gfx/layers/PaintThread.cpp:178:38
    #13 0x7f164101e0e3 in mozilla::detail::RunnableFunction<mozilla::layers::PaintThread::QueuePaintTask(mozilla::UniquePtr<mozilla::layers::PaintTask, mozilla::DefaultDelete<mozilla::layers::PaintTask> >&&)::$_7>::Run() src/objdir-ff-ubsan/dist/include/nsThreadUtils.h:564
    #14 0x7f163dbd7722 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1225:14
    #15 0x7f163dbde3c6 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
    #16 0x7f163f10169c in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:333:5
    #17 0x7f163ef73c17 in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #18 0x7f163ef73c17 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #19 0x7f163ef73c17 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #20 0x7f163dbd09c9 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:458:11
    #21 0x7f16627f4ff9 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:198:5
    #22 0x7f16624466da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #23 0x7f166142488e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?
Priority: -- → P3
Priority: P3 → P1
Assignee: nobody → jbonisteel

Hi Lee - does this bug look actionable? Any suggestions?

Flags: needinfo?(lsalzman)

Jeff do you have any suggestions for this bug?

Flags: needinfo?(jmuizelaar)

Looks like ColorComponentAtPoint is doing absolutely nothing to clamp offset samples within the bounds of the surface, which seems like a recipe for disaster.

Flags: needinfo?(lsalzman)

Lee - let me know if you can take this one on!

Assignee: jbonisteel → lsalzman
Flags: needinfo?(lsalzman)

On closer inspection, I don't believe there is any actual security vulnerability going on here.

The sampling bounds are inflated to compensate for the boundary cases, and what the overflow check here is finding is that a legitimate signed negative offset (that none the less stays within the data bounds of the suface) which gets cast to an unsigned size_t value (and due to properties of 2s complement, has the same effect under addition with a pointer) before getting added to the pointer.

So other than casting the value to a ptrdiff_t to maybe silence the warning if it really is essentially to silence it, there is nothing that need be done.

Flags: needinfo?(lsalzman)
Flags: needinfo?(jmuizelaar) → needinfo?(dveditz)

Adding a cast to silence this to prevent tripping over it again in the future would be great.

Group: gfx-core-security
Type: defect → task
Flags: needinfo?(dveditz)
Summary: addition of unsigned offset overflowed in [@ mozilla::gfx::Point3DTyped] → Silence ubsan unsigned offset overflowed false-positive in [@ mozilla::gfx::Point3DTyped]
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5ebaf7bd462
silence filter node type warnings. r=jrmuizel
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.