1678938 - Assertion `numClip < nump + 2' failed.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Brian Carpenter [:geeknik]]

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)

Comment 1

[Kelsey Gilbert [:jgilbert]]

Do you have a testcase in particular you can post?

Comment 2

[Brian Carpenter [:geeknik]]

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

Comment 3

[Brian Carpenter [: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.

Comment 4

[Kelsey Gilbert [:jgilbert]]

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.

Comment 5

[Brian Carpenter [:geeknik]]

Attachment: rucss_0_632.html

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

Comment 6

[Tyson Smith [:tsmith]]

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

Comment 7

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 8

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Comment 9

[Tyson Smith [:tsmith]]

#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

Comment 10

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/Hk-LYfOSJnxMlthyn1UzHA/index.html

Comment 11

[Daniel Veditz [:dveditz]]

What kind of consequences is this assert protecting us from?

Comment 12

[Kelsey Gilbert [:jgilbert]]

(over to the WR team)

Comment 13

[Glenn Watson [:gw]]

Looks like something in swgl from the stack trace?

Comment 14

[Lee Salzman [: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?

Comment 15

[Lee Salzman [:lsalzman]]

Attachment: Bug 1678938 - nudge clipped coordinates inside clip boundary. r?jimb

Depends on D99007

Comment 16

[Andrew McCreight [:mccr8]]

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

Comment 17

[Daniel Veditz [:dveditz]]

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?

Comment 18

[Lee Salzman [:lsalzman]]

(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 19

[Lee Salzman [:lsalzman]]

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.

Comment 20

[Lee Salzman [:lsalzman]]

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.

Comment 21

[Andrew McCreight [:mccr8]]

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.

Comment 22

[Lee Salzman [:lsalzman]]

(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!

Comment 23

[Lee Salzman [:lsalzman]]

Attachment: Bug 1678938 - flip the clip nudge direction when crossing back outside. r?jimb

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This had landed: https://hg.mozilla.org/integration/autoland/rev/b022f8da97725c57f2cc7860981f8ca1f5c48fd4

Backed out for reftest failures at 1157984-1.html:

https://hg.mozilla.org/integration/autoland/rev/dd4d7d2e38e20f74fe0534d431d6bd50545a3e91

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=b022f8da97725c57f2cc7860981f8ca1f5c48fd4&selectedTaskRun=dyqjk_suQYyRW_G3DBxARA.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=324327334&repo=autoland

REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/transform-3d/1157984-1.html != about:blank | image comparison, max difference: 0, number of differing pixels: 0

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

flip the clip nudge direction when crossing back outside. r=jimb
https://hg.mozilla.org/integration/autoland/rev/9325142a60216c7bee1403d09c6cd4bd0a52b47e
https://hg.mozilla.org/mozilla-central/rev/9325142a6021

Comment 27

[Daniel Veditz [:dveditz]]

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?

Comment 28

[Lee Salzman [: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.

Comment 29

[BugBot [:suhaib / :marco/ :calixte]]

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.