1743767 - (CVE-2022-31737) heap-buffer-overflow in mozilla::gl::GLContext::raw_fReadPixels

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P1)

Description

[Atte Kettunen]

Attachment: heap-buffer-overflow-mozilla-glGLContextraw_fReadPixels.html

Tested on:
OS: Ubuntu 20.04 in VMware VM
Firefox:

fuzzfetch --target firefox --os Linux --asan --fuzzing -n firefox
[2021-12-01 10:14:05] Identified task: https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/gecko.v2.mozilla-central.latest.firefox.linux64-fuzzing-asan-opt
[2021-12-01 10:14:05] > Task ID: Frxzk0u7Qeu4nG7rilMWAQ
[2021-12-01 10:14:05] > Rank: 1638335107
[2021-12-01 10:14:05] > Changeset: 89800efd9e5cfcf0146767961a63d5c4e2a86e2c
[2021-12-01 10:14:05] > Build ID: 20211201050507
[2021-12-01 10:14:05] > Downloading: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Frxzk0u7Qeu4nG7rilMWAQ/artifacts/public/build/target.tar.bz2 (382.95MiB total)

prefs.js: fuzzing prefs.js generated with prefpicker, adding as an attachment.

I have reproduced the issue on my machine with/without the fuzzing prefs.js, and with/without Xvfb . Including prefs.js as an attachment to ensure reproducibility.

The write size can be changed by changing the canvas width. For example changing width and 300px, the write will be size 1200, 10px and it will be 40. 1px on fuzzfetch --fuzzing build will result to a

Hit MOZ_CRASH(Trying to run a non-debug fuzzing build.) at /builds/worker/checkouts/gecko/third_party/rust/neqo-crypto/src/aead_fuzzing.rs:19

I don't think it is related to the issue. Using 1px with regular asan-build downloaded without --fuzzing doesn't crash, but other tested values 10px up reproduced sameway on regular and fuzzing builds.

Reproducing:

$ ffpuppet -p ~/prefs.js -u ./heap-buffer-overflow-mozilla-glGLContextraw_fReadPixels.html ./firefox/firefox --xvfb -d
[2021-12-01 10:37:01] Launching Firefox...
[2021-12-01 10:37:13] Running Firefox (pid: 16680)...
[2021-12-01 10:37:14] Shutting down...
[2021-12-01 10:37:16] Firefox process is closed. (Reason: <Reason.ALERT: 0>)
[2021-12-01 10:37:16] Displaying logs...
===
=== Dumping 'log_ffp_asan_16655.log.16807.txt' (14.89KB)
===
=================================================================
==16807==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f59218c8c00 at pc 0x55ee081ba203 bp 0x7fff0cc77200 sp 0x7fff0cc769c0
WRITE of size 1600 at 0x7f59218c8c00 thread T0 (file:// Content)
    #0 0x55ee081ba202 in __interceptor_memcpy /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:827:5

ASAN-trace(truncated, full trace as an attachment):

==16807==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f59218c8c00 at pc 0x55ee081ba203 bp 0x7fff0cc77200 sp 0x7fff0cc769c0
WRITE of size 1600 at 0x7f59218c8c00 thread T0 (file:// Content)
    #0 0x55ee081ba202 in __interceptor_memcpy /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:827:5
    #1 0x7f59333e3ef8  (/usr/lib/x86_64-linux-gnu/dri/swrast_dri.so+0x320ef8)
    #2 0x7f59332997fc  (/usr/lib/x86_64-linux-gnu/dri/swrast_dri.so+0x1d67fc)
    #3 0x7f59333e42dc  (/usr/lib/x86_64-linux-gnu/dri/swrast_dri.so+0x3212dc)
    #4 0x7f59333e4b15  (/usr/lib/x86_64-linux-gnu/dri/swrast_dri.so+0x321b15)
    #5 0x7f59c93ecd70 in mozilla::gl::GLContext::raw_fReadPixels(int, int, int, int, unsigned int, unsigned int, void*) /builds/worker/workspace/obj-build/dist/include/GLContext.h:1568:5
    #6 0x7f59cc103cb1 in mozilla::WebGLContext::DoReadPixelsAndConvert(mozilla::webgl::FormatInfo const*, mozilla::webgl::ReadPixelsDesc const&, unsigned long, unsigned long, unsigned int) /builds/worker/checkouts/gecko/dom/canvas/WebGLContextGL.cpp
    #7 0x7f59cc106190 in mozilla::WebGLContext::ReadPixelsImpl(mozilla::webgl::ReadPixelsDesc const&, unsigned long, unsigned long) /builds/worker/checkouts/gecko/dom/canvas/WebGLContextGL.cpp:1238:7
    #8 0x7f59cc1041a7 in mozilla::WebGLContext::ReadPixelsInto(mozilla::webgl::ReadPixelsDesc const&, mozilla::Range<unsigned char> const&) /builds/worker/checkouts/gecko/dom/canvas/WebGLContextGL.cpp:962:10
    #9 0x7f59cbfe89b4 in mozilla::HostWebGLContext::ReadPixelsInto(mozilla::webgl::ReadPixelsDesc const&, mozilla::Range<unsigned char> const&) const /builds/worker/checkouts/gecko/dom/canvas/HostWebGLContext.h:646:22
...
    #53 0x55ee08252dfd in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #54 0x55ee08253228 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:327:18
    #55 0x7f59e54b50b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #56 0x55ee081a1ec9 in _start (/firefox/firefox+0x5cec9)

0x7f59218c8c00 is located 0 bytes to the right of 640000-byte region [0x7f592182c800,0x7f59218c8c00)
allocated by thread T0 (file:// Content) here:
    #0 0x55ee0821e632 in __interceptor_calloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:138:3
    #1 0x7f59d39151cb in AllocateArrayBufferContents /builds/worker/checkouts/gecko/js/src/vm/ArrayBufferObject.cpp:459:11
    #2 0x7f59d39151cb in std::tuple<js::ArrayBufferObject*, unsigned char*> js::ArrayBufferObject::createBufferAndData<(js::ArrayBufferObject::FillContents)0>(JSContext*, unsigned long, js::AutoSetNewObjectMetadata&, JS::Handle<JSObject*>) /builds/worker/checkouts/gecko/js/src/vm/ArrayBufferObject.cpp:1351:18
    #3 0x7f59d390ceea in js::ArrayBufferObject::createZeroed(JSContext*, unsigned long, JS::Handle<JSObject*>) /builds/worker/checkouts/gecko/js/src/vm/ArrayBufferObject.cpp:1414:7
...
    #22 0x7f59ce5744aa in mozilla::dom::ScriptElement::MaybeProcessScript() /builds/worker/checkouts/gecko/dom/script/ScriptElement.cpp:118:18
    #23 0x7f59c91a2d6e in nsIScriptElement::AttemptToExecute() /builds/worker/workspace/obj-build/dist/include/nsIScriptElement.h:211:18

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:827:5 in __interceptor_memcpy
Shadow bytes around the buggy address:
...
==16807==ABORTING

Comment 1

[Atte Kettunen]

Attachment: prefs.js

Attaching the prefs.js used to reproduce, if prefpicker generated doesn't reproduce clearly.

Comment 2

[Atte Kettunen]

Attachment: log_ffp_asan_21682.log.21844.txt

Full AddressSanitizer output.

Comment 3

[Andrew McCreight [:mccr8]]

The prefs include webgl.enable-draft-extensions and webgl.enable-privileged-extensions, so maybe these aren't standard web-exposed APIs being used. (I haven't looked at the test case.)

Comment 4

[Atte Kettunen]

The issue reproduces without usage of the prefs.js. Also I tested that it reproduces with the mentioned webgl.enable-draft-extensions and webgl.enable-privileged-extensions set to false, from the attached prefs.js.

Comment 5

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/DVAXc3npneKx7gxWx-6hxA/index.html

Comment 6

[Kelsey Gilbert [:jgilbert]]

So the y=-1 is valid, but we're actually writing too far, rather than too early.
Our code to handle this is super wrong, and just resets to row0 but doesn't decrease the amount it reads, so it tries to read 400x400px starting at a 100x1px offset into the destination buffer, which gladly writes off the end of that buffer.

We need better ranged types here. We cast Range into uintptr_t too early, but unfortunately that's because we need to handle both the void* and uintptr_t-into-pbo usecase.
Sending a void* that doesn't point to valid memory into glReadPixels is actually super against the rules in c++ (pointers must point within a valid range of their types, with one element of slack on either end for end/reverse-end), so we try to minimize the amount of code we have that treats this pbo offset as a void*.
We could go in-for-a-penny-in-for-a-pound and use a Range<uint8_t>[0,pbo->ByteSize()], but that seems like tempting fate.

Instead I'm making a RangedInt<T> struct with CheckedInt<T>, that we can use as RangedInt<uintptr_t>.
Writing this code, it actually makes some of the bounds checks much easier, so win-win.

Comment 7

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

The severity field is not set for this bug.
:jgilbert, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Kelsey Gilbert [:jgilbert]]

Please retest now that we've landed bug 1750310.

Comment 9

[Atte Kettunen]

Still reproduces.

From mozilla-central log, it looks like the patch from bug #1750310 is included in the version that fuzzfetch downloads.

:$ fuzzfetch --target firefox --central --os Linux --asan --fuzzing -n firefox
[2022-02-24 23:42:17] Identified task: https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/gecko.v2.mozilla-central.latest.firefox.linux64-fuzzing-asan-opt
[2022-02-24 23:42:17] > Task ID: OuiYrcUqRpyXyRqT52NfQQ
[2022-02-24 23:42:17] > Rank: 1645739161
[2022-02-24 23:42:17] > Changeset: fb443d9a5f9cfaa17acc81c25473d7093d5cf696
[2022-02-24 23:42:17] > Build ID: 20220224214601
[2022-02-24 23:42:17] > Downloading: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/OuiYrcUqRpyXyRqT52NfQQ/artifacts/public/build/target.tar.bz2 (436.42MiB total)
[2022-02-24 23:42:31] .. downloaded (32.73MB/s)
[2022-02-24 23:42:31] .. extracting
[2022-02-24 23:46:07] Extracted into /firefox

Reproducing:

:$ ffpuppet ./firefox/firefox -u ./heap-buffer-overflow-mozilla-glGLContextraw_fReadPixels.html -d
[2022-02-24 23:50:10] Launching Firefox...
[2022-02-24 23:50:23] Running Firefox (pid: 3217697)...
[2022-02-24 23:50:25] Shutting down...
[2022-02-24 23:50:28] Firefox process is closed. (Reason: ALERT)
[2022-02-24 23:50:28] Displaying logs...
===
=== Dumping 'log_ffp_asan_3217695.log.3217783.txt' (16.62KB)
===
=================================================================
==3217783==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fe68cffac00 at pc 0x55a99b0c1c13 bp 0x7ffe42bd5ab0 sp 0x7ffe42bd5270
WRITE of size 1600 at 0x7fe68cffac00 thread T0 (file:// Content)
    #0 0x55a99b0c1c12 in __interceptor_memcpy /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:827:5
    #1 0x7fe694dfa758  (/usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so+0x39a758)
    #2 0x7fe694cb44cb  (/usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so+0x2544cb)
    #3 0x7fe694dfab34  (/usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so+0x39ab34)
    #4 0x7fe694dfb385  (/usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so+0x39b385)
    #5 0x7fe6af901950 in mozilla::gl::GLContext::raw_fReadPixels(int, int, int, int, unsigned int, unsigned int, void*) /builds/worker/workspace/obj-build/dist/include/GLContext.h:1568:5
    #6 0x7fe6b28f1953 in mozilla::WebGLContext::DoReadPixelsAndConvert(mozilla::webgl::FormatInfo const*, mozilla::webgl::ReadPixelsDesc const&, unsigned long, unsigned long, unsigned int) /builds/worker/checkouts/gecko/dom/canvas/WebGLContextGL.cpp
...

Comment 10

[Kelsey Gilbert [:jgilbert]]

Thanks!

Comment 11

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:jgilbert, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 12

[Kelsey Gilbert [:jgilbert]]

S2 is probably right for this, though it's primarily on Mesa it seems?

Comment 13

[Kelsey Gilbert [:jgilbert]]

Well it doesn't repro on this system anymore. I wonder if this is a different machine than the one I investigated it on originally?

Comment 14

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1743767 - Tighten bounds and add asserts for row-by-row ReadPixels.

Comment 16

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9274154 [details]
Bug 1743767 - Tighten bounds and add asserts for row-by-row ReadPixels.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Tricky but not impossible.
  I'm definitely adding asserts in a patch to a bug that's restricted-access, though I don't say exactly what the problem is. I don't know how to answer "is this a bullseye on the security problem", so I said "no", because there's still some thinking required of any adversary looking at this.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should be pretty easy.
• How likely is this patch to cause regressions; how much testing does it need?: unlikely if it gets through CI
• Is Android affected?: Yes

Comment 17

[Tom Ritter [:tjr]]

Comment on attachment 9274154 [details]
Bug 1743767 - Tighten bounds and add asserts for row-by-row ReadPixels.

Let's land and uplift this one mid-next week, on May 11.

Comment 18

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

Tighten bounds and add asserts for row-by-row ReadPixels. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/4a4eec75fcd72e9f87e54a45a4ff2c809d0dcd00
https://hg.mozilla.org/mozilla-central/rev/4a4eec75fcd7

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9274154 [details]
Bug 1743767 - Tighten bounds and add asserts for row-by-row ReadPixels.

This needs a rebased patch for ESR91.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Landed for 101.0b6.
https://hg.mozilla.org/releases/mozilla-beta/rev/832630d39630

Comment 21

[Kelsey Gilbert [:jgilbert]]

Attachment: [esr91 backport] 0001-Bug-1743767-esr91-Tighten-bounds-and-add-asserts-for.patch

Comment 22

[Kelsey Gilbert [:jgilbert]]

I can't build esr91 on Windows or Mac, and I can't seem to push it to Try either, so here's the patch I guess? I hope it builds!

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9276352 [details]
[esr91 backport] 0001-Bug-1743767-esr91-Tighten-bounds-and-add-asserts-for.patch

Busted on Try:
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=d65aef008c21ee597b78aea149d1bc8d53ad89c2

FWIW, I find for ESR, the easiest way to get a Try push going is to use |./mach try empty| and add jobs to the push via TH.

Comment 24

[Kelsey Gilbert [:jgilbert]]

Thanks! I'm trying that.

Comment 25

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Bug-1743767-esr91-Tighten-bounds-and-add-asserts-for.patch

Seems green:
https://treeherder.mozilla.org/jobs?repo=try&revision=2a204bec20c1858c5eedf8f34a2a4f8903de30ed

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9276496 [details] [diff] [review]
0001-Bug-1743767-esr91-Tighten-bounds-and-add-asserts-for.patch

Approved for 91.10esr. Thanks for the quick rebase!

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/00f5d50b71c6

Comment 28

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 29

[Jim Blandy :jimb]

Hit MOZ_CRASH(Trying to run a non-debug fuzzing build.) at /builds/worker/checkouts/gecko/third_party/rust/neqo-crypto/src/aead_fuzzing.rs:19

This is bug 1743672, for reference.