Closed Bug 1678938 Opened 4 years ago Closed 4 years ago

Assertion `numClip < nump + 2' failed.

Categories

(Core :: Graphics: WebRender, defect)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
thunderbird_esr78 --- unaffected
firefox-esr78 --- unaffected
firefox83 --- unaffected
firefox84 --- unaffected
firefox85 --- fixed

People

(Reporter: geeknik, Assigned: lsalzman)

References

Details

(6 keywords, Whiteboard: [sec-survey])

Attachments

(5 files)

Whilst fuzzing Firefox ASan Nightly (Build ID 20201122093438) with rucss, we discovered the following assertion failure. Marking as a security bug until someone smarter than us says otherwise because of our recent WebGL findings.

For some reason, ASAN isn't catching this and generating a crash dump. This is all I have from the console, but it's 100% repeatable:

firefox: src/gl.cc:3732: int clip_side(int, Point3D *, glsl::Interpolants *, Point3D *, glsl::Interpolants *) [AXIS = glsl::Y]: Assertion `numClip < nump + 2' failed.
Exiting due to channel error.
Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=68.5117) Exiting due to channel error.
Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=69.5414) Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=68.6727) Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.

STR:

  1. New Nightly Profile
  2. enable the following:
    pdfjs.enableWebGL, webgl.all-angle-options, webgl.allow-fb-invalidation, webgl.allow-immediate-queries, webgl.angle.force-warp, webgl.enable-ahardwarebuffer, webgl.enable-privileged-extensions, webgl.force-enabled, webgl.out-of-process, webgl.out-of-process.force
  3. git clone https://github.com/roddux/rucss && rucss/src/webhandler.py -p 8080
  4. Launch Nightly and point it at localhost:8080
  5. Grab a coffee (probably will crash before you get back to your seat)
Flags: sec-bounty?
Group: core-security → gfx-core-security

Do you have a testcase in particular you can post?

Flags: needinfo?(geeknik)

No, but only because Build ID 20201122093438 crashes within seconds of connecting to localhost with this particular fuzzer.

Flags: needinfo?(geeknik)

#2 of my STR is no longer relevant. Just having a clean profile is enough to trigger the assertion. Trying to come up with a reproducible test case.

Thanks, that's appreciated! It's easier for me to trust a testcase I can skim than a whole fuzzing framework repo! :)

And thanks for testing with a clean profile, that's good data.

Attached file rucss_0_632.html (deleted) —

This html will crash Build ID 20201123095316 instantly with the reported assertion failure.

This was first seen by fuzzers @ Fri, 27 Nov 2020 01:22:13 +0000

Attached file testcase.html (deleted) —
Attached file prefs.js (deleted) —
Flags: in-testsuite?
Keywords: crash, testcase
#0 0x7f9c84e3bf47 in raise /build/glibc-2ORdQG/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51
#1 0x7f9c84e3d8b0 in abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:79
#2 0x7f9c84e2d429 in __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:92
#3 0x7f9c84e2d4a1 in __assert_fail /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:101
#4 0x7f9c7121302c in draw_quad(int, Texture&, int, Texture&) (m-c-20201126212448-fuzzing-asan-opt/libxul.so+0x13e4d02c)
#5 0x7f9c7120b221 in DrawElementsInstanced (m-c-20201126212448-fuzzing-asan-opt/libxul.so+0x13e45221)
#6 0x7f9c70d72e2e in webrender::device::gl::Device::draw_indexed_triangles_instanced_u16::heff8285dbad34772 src/gfx/wr/webrender/src/device/gl.rs:3407:9
#7 0x7f9c70d72e2e in webrender::renderer::Renderer::draw_instanced_batch::hec2f6afb99bbc029 src/gfx/wr/webrender/src/renderer.rs:4328:13
#8 0x7f9c70d6ef8d in webrender::renderer::Renderer::draw_alpha_batch_container::hf06ac38b11e641cb src/gfx/wr/webrender/src/renderer.rs:4780:17
#9 0x7f9c70d60b2b in webrender::renderer::Renderer::draw_picture_cache_target::h403f6db188df66d1 src/gfx/wr/webrender/src/renderer.rs:4597:9
#10 0x7f9c70d60b2b in webrender::renderer::Renderer::draw_frame::ha48e1a1bfdee5c82 src/gfx/wr/webrender/src/renderer.rs:6358:21
#11 0x7f9c70d80dc8 in webrender::renderer::Renderer::render_impl::h58e3e2b148b51f6a src/gfx/wr/webrender/src/renderer.rs:3663:17
#12 0x7f9c70d8e297 in webrender::renderer::Renderer::render::h5cc5e23ee60b3143 src/gfx/wr/webrender/src/renderer.rs:3414:30
#13 0x7f9c70f5a94b in wr_renderer_render src/gfx/webrender_bindings/src/bindings.rs:614:11
#14 0x7f9c64c5a04d in mozilla::wr::RendererOGL::UpdateAndRender(mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*, mozilla::wr::RendererStats*) src/gfx/webrender_bindings/RendererOGL.cpp:193:8
#15 0x7f9c64c58306 in mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> const&, mozilla::TimeStamp const&, bool, mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*) src/gfx/webrender_bindings/RenderThread.cpp:488:31
#16 0x7f9c64c5737f in mozilla::wr::RenderThread::HandleFrameOneDoc(mozilla::wr::WrWindowId, bool) src/gfx/webrender_bindings/RenderThread.cpp:325:3
#17 0x7f9c64c6f156 in applyImpl<mozilla::wr::RenderThread, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool), StoreCopyPassByConstLRef<mozilla::wr::WrWindowId>, StoreCopyPassByConstLRef<bool> , 0, 1> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1096:12
#18 0x7f9c64c6f156 in apply<mozilla::wr::RenderThread, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool)> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1102:12
#19 0x7f9c64c6f156 in mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool), true, (mozilla::RunnableKind)0, mozilla::wr::WrWindowId, bool>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1148:13
#20 0x7f9c62d00c2d in MessageLoop::RunTask(already_AddRefed<nsIRunnable>) src/ipc/chromium/src/base/message_loop.cc:465:9
#21 0x7f9c62d01ace in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:473:5
#22 0x7f9c62d0234b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:548:13
#23 0x7f9c62d039c6 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
#24 0x7f9c62d007f1 in RunInternal src/ipc/chromium/src/base/message_loop.cc:334:10
#25 0x7f9c62d007f1 in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#26 0x7f9c62d007f1 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#27 0x7f9c62d21228 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:191:16
#28 0x7f9c62d1313c in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
#29 0x7f9c85f406da in start_thread /build/glibc-2ORdQG/glibc-2.27/nptl/pthread_create.c:463
#30 0x7f9c84f1ea3e in clone /build/glibc-2ORdQG/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

What kind of consequences is this assert protecting us from?

Flags: needinfo?(jgilbert)
Component: Canvas: WebGL → Graphics: WebRender

(over to the WR team)

Flags: needinfo?(jgilbert) → needinfo?(gwatson)

Looks like something in swgl from the stack trace?

Flags: needinfo?(gwatson) → needinfo?(lsalzman)

So it does look like something that can potentially cause a buffer to overflow, but not with data that is easily predicted and only a finite amount of data on the SWGL stack. Also, it is worth noting the Software WebRender is only deployed behind a pref for releases, and otherwise is only actually prefed on by default for Nightly Linux users. What would be appropriate sec classification here considering exposure to this is not in release?

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

Depends on D99007

Assignee: nobody → lsalzman
Status: NEW → ASSIGNED

For rating purposes, we treat things that are Nightly-only the same as things that are on release.

What Andrew said in comment 16. It keeps the bugs with appropriate scrutiny so they don't sneak unnoticed and unfixed into Release, plus it's the appropriate risk rating for the tens of thousands of people who are using Nightly. We use the status fields so release management knows how excited to get about any given sec-high bug.

Is this feature going to ride the trains to release in 85? If not disabled might be a better status than affected. Similarly, unless this is a recent regression is disabled appropriate for some of the older releases currently marked unaffected?

Flags: needinfo?(dveditz)
Keywords: sec-high

(In reply to Daniel Veditz [:dveditz] from comment #17)

What Andrew said in comment 16. It keeps the bugs with appropriate scrutiny so they don't sneak unnoticed and unfixed into Release, plus it's the appropriate risk rating for the tens of thousands of people who are using Nightly. We use the status fields so release management knows how excited to get about any given sec-high bug.

Is this feature going to ride the trains to release in 85? If not disabled might be a better status than affected. Similarly, unless this is a recent regression is disabled appropriate for some of the older releases currently marked unaffected?

Software WebRender is not going to ride the trains as of yet. The current exposure of this bug, based on telemetry, is about potentially only 200 nightly users total.

Comment on attachment 9191809 [details]
Bug 1678938 - nudge clipped coordinates inside clip boundary. r?jimb

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The overflow is confined to fairly to a stack buffer near the top of the stack in the GPU/compositor process. And the data would have to be constructed such that given a triangle specified by three 3-dimensional points, once clipped against 6 clipping planes, yield a small finite number of points in floating point format that would then overwrite some data at the top of the stack.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: 85
  • If not all supported branches, which bug introduced the flaw?: Bug 1677293
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: So far Software WebRender is only really available if you enable the gfx.webrender.software pref which is off by default for most users. Recently in bug 1677293 we enabled SW-WR by default for only a subset the Nightly Linux population (which will not ride the trains) and which we expect exposure of several hundred users at most. This does not effect beta/release and it is recent enough that I believe just quickly fixing it before nightlies with the bug have chance to spread far and wide should adequately address it.
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely to cause any serious regressions for reasons noted above since we don't actually enable SW-WR beyond nightly.
Attachment #9191809 - Flags: sec-approval?

I would appreciate if we could get a sec-approval and get this fix landed today before we roll out to a larger Nightly population.

Flags: needinfo?(dveditz)

Comment on attachment 9191809 [details]
Bug 1678938 - nudge clipped coordinates inside clip boundary. r?jimb

If this pref is only active for Nightly, it does not need sec approval.

Flags: needinfo?(dveditz)
Attachment #9191809 - Flags: sec-approval?

(In reply to Andrew McCreight [:mccr8] from comment #21)

Comment on attachment 9191809 [details]
Bug 1678938 - nudge clipped coordinates inside clip boundary. r?jimb

If this pref is only active for Nightly, it does not need sec approval.

Okay, thanks!

nudge clipped coordinates inside clip boundary. r=jimb
https://hg.mozilla.org/integration/autoland/rev/a8a797ccbe64e92af06b37491b827148b898c925
https://hg.mozilla.org/mozilla-central/rev/a8a797ccbe64

The backout was for:
flip the clip nudge direction when crossing back outside. r=jimb

Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch

Software WebRender is not going to ride the trains as of yet. The current exposure of this bug, based on telemetry, is about potentially only 200 nightly users total.

For bounty purposes it doesn't matter if it's a nightly-only feature that's not yet riding the trains, but is it ON by default? Currently that appears to be "no". We're running experiments to see if it's stable enough even for nightly folks. Is that right?

Flags: needinfo?(lsalzman)

(In reply to Daniel Veditz [:dveditz] from comment #27)

Software WebRender is not going to ride the trains as of yet. The current exposure of this bug, based on telemetry, is about potentially only 200 nightly users total.

For bounty purposes it doesn't matter if it's a nightly-only feature that's not yet riding the trains, but is it ON by default? Currently that appears to be "no". We're running experiments to see if it's stable enough even for nightly folks. Is that right?

Correct. It's not on by default yet for any general population. We're just doing an experiment with only a subset of the nightly population to toggle it on and see if any major bugs come up. Only after we get feedback from that subset are we later going to pref it on for the general population.

Flags: needinfo?(lsalzman)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(lsalzman)
Whiteboard: [sec-survey]
Flags: needinfo?(lsalzman)
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: