1328762 - (CVE-2017-5031) Invalid read @ libGLESv2.dll!rx::Image11::disassociateStorage() | Assertion failure: !err

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Bob Clary [:bc] (inactive)]

Windows 7 32bit show a EXCEPTION_ACCESS_VIOLATION_READ crash rated high exploitable on 

https://sketchfab.com/models/7346263a68554cb3b536a872d5868374
https://sketchfab.com/models/9958172a6fd74610922a3a5fcb3e0850

Both Windows 7 and 10 32bit also show OOM crashes so you may need to work at it to get the actual Angle crash.

I also see the following on 32bit Windows.

Assertion failure: !err, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/src/dom/canvas/WebGLContextDraw.cpp:739

The stacks for the assertions are short, symbol-less and pretty useless.

Nightly/53 Linux 64bit showed a nullptr crash

Operating system: Linux
                  0.0.0 Linux 4.8.15-300.fc25.x86_64 #1 SMP Thu Dec 15 23:10:23 UTC 2016 x86_64
CPU: amd64
     family 6 model 45 stepping 2
     2 CPUs

GPU: UNKNOWN

Crash reason:  SIGSEGV
Crash address: 0x0
Process uptime: not available

Thread 0 (crashed)
 0  libxul.so!mozilla::WebGLTexture::TexImage [WebGLTextureUpload.cpp:45f796204c54 : 1303 + 0x24]
    rax = 0x0000000000000000   rdx = 0x0000000000000000
    rcx = 0x0000000000000b40   rbx = 0x00007f050f1d0000
    rsi = 0x00007f0546f43730   rdi = 0x00007f0538bc5b8a
    rbp = 0x00007ffca2ceae00   rsp = 0x00007ffca2cead50
     r8 = 0x00007f0546f43730    r9 = 0x00007f0548020740
    r10 = 0x0000000000000073   r11 = 0x0000000000000000
    r12 = 0x00007f050f1c9c80   r13 = 0x00007ffca2ceafb5
    r14 = 0x00007ffca2cead88   r15 = 0x00007f050f1935a8
    rip = 0x00007f05368aac97

Comment 1

[Daniel Veditz [:dveditz]]

I could not reproduce with Win7 32-bit Firefox Beta (51) or Release (50.0). Given you mention an assert I guess you're running a debug build?

Raymond: any luck getting ASAN Windows builds? If so please try the links in comment 0

Comment 2

[Daniel Veditz [:dveditz]]

Possibly related to bug 1325511? Have no idea if "Buffer11" storage is related to "Image11" storage. Is "11" a thing, or just a version number?

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

It's DirectX 11, so it's always going to be that (sort of), and could be related to bug 1325511.

Comment 4

[Daniel Veditz [:dveditz]]

needinfo raymond for comment 1

Comment 5

[Raymond Forbes[:rforbes]]

So far I have not seen a crash. However, we build ASAN windows in 64 bit as 32 bit doesn't really work.

Comment 6

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Bob, are you able to reproduce this anymore?

Comment 7

[Bob Clary [:bc] (inactive)]

Yes. I re-submitted the over 100 sketchfab urls and reproduced pretty much the same set of behaviors. This site does cause *lots* of OOM errors on 32bit but for whatever reason *sometimes* it causes us to fall down and...

exploitable high:
rx::Image11::disassociateStorage rx::Image11::redefine rx::TextureD3D_2D::redefineImage rx::TextureD3D_2D::initMipmapImages rx::TextureD3D::generateMipmap EXCEPTION_ACCESS_VIOLATION_READ	0xffffffffe5e5e621

7 times on 32bit Windows 7.

https://sketchfab.com/models/3109e582b2bd4f689ea682a9abca4e41 beta/aurora/nightly

https://sketchfab.com/models/3a6d61475d8349f7b2cd84127d8b2129 beta

https://sketchfab.com/models/434d0a895c31481a8a6fb84adfbec0c1 aurora

plus the standard Assertion failure: !err and WebGL related crashes and assertions.

Note: I tried to spider the site from my Fedora 25 4.10.5 workstation and locked up the user interface pretty hard.

Comment 8

[Bob Clary [:bc] (inactive)]

Attachment: example log output

Comment 9

[Bob Clary [:bc] (inactive)]

Attachment: example crash report with eax = 0xe5e5e5e5

Comment 10

[Andrew McCreight [:mccr8]]

Milan, do you know of somebody who could look into this? Thanks.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 53 since this is sec-critical.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Jeff, is this covered by any of the outstanding ANGLE patches?

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Milan, anyone else who can poke around in ANGLE and have a look here? 
I'll keep this tracked and marked as affected for 53 in case we end up with a 53 dot release.

Comment 14

[Milan Sreckovic [:milan] (needinfo for best results)]

We are updating to ANGLE as we speak, we should check this once we have that update.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Chatted with Milan, sounds like this change in ANGLE will be riding with 55.

Comment 16

[Frederik Braun [:freddy]]

It seems only bc managed to reproduce. Can you re-test in nightly to verify whether this is indeed fixed by the ANGLE work as Milan suggested, Bob?

Comment 17

[Bob Clary [:bc] (inactive)]

I tested the 200+ sketchfab urls on Fedora 25, Ubuntu 16.04 64bit, Windows 7, 10 32bit and 64bit on Beta/54 and Nightly/55.

I found 2 crashes on window 7 32bit beta/54 opt

rx::Image11::disassociateStorage rx::Image11::redefine rx::TextureD3D_2D::redefineImage rx::TextureD3D_2D::initMipmapImages rx::TextureD3D::generateMipmap

That were rated high exploitable. I did not reproduce this on nightly/55 however.

I also saw lots of mozalloc_abort | mozalloc_handle_oom | moz_xmalloc std::_Allocate std::vector<unsigned char, std::allocator<unsigned char> >::_Reallocate rx::d3d11::GenerateInitialTextureData rx::Image11::createStagingTexture 

on beta opt Windows 7 and 10 32bit.

ucrtbase.dll ucrtbase.dll ucrtbase.dll ucrtbase.dll rx::Image11::createStagingTexture

on beta debug and nightly Windows 7 32bit.

and one beta debug windows 7 ucrtbase.dll ucrtbase.dll ucrtbase.dll ucrtbase.dll rx::UniformStorage11::UniformStorage11

I didn't see any other exploitable rated crashes with the sketchfab urls.

Looks like the Angle update resolved this. I'll let you resolve the bug how you like.

Comment 18

[Bob Clary [:bc] (inactive)]

Have we been outed by Chrome's fix: https://www.us-cert.gov/ncas/bulletins/SB17-121 	CVE-2017-5031

Comment 19

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Bob Clary [:bc:] from comment #18)
> Have we been outed by Chrome's fix:
> https://www.us-cert.gov/ncas/bulletins/SB17-121 	CVE-2017-5031

Liz, Daniel, do we need a chemspill or some such?

Comment 20

[Andrew McCreight [:mccr8]]

For what it is worth, the Chrome bug is public and has a test case, so somebody could see what that does in Firefox: https://bugs.chromium.org/p/chromium/issues/detail?id=682020

Comment 21

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #20)
> For what it is worth, the Chrome bug is public and has a test case, so
> somebody could see what that does in Firefox:
> https://bugs.chromium.org/p/chromium/issues/detail?id=682020

Though the patch is in Windows code, so it would require a working Windows ASan build.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

We can be ready for chemspill or to include a fix in a dot release, once we have a patch here. 
Do you want to try updating ANGLE, or try to cherry pick a fix ?

Comment 23

[Milan Sreckovic [:milan] (needinfo for best results)]

Started a conversation with :jgilbert, we'll continue it in here.  For cherry picking, it may help us to know which Google bug fixed it, so that it can lead us to a patch, instead of trying to reverse engineer it.

Comment 24

[Andrew McCreight [:mccr8]]

(In reply to Milan Sreckovic [:milan] from comment #23)
> Started a conversation with :jgilbert, we'll continue it in here.  For
> cherry picking, it may help us to know which Google bug fixed it, so that it
> can lead us to a patch, instead of trying to reverse engineer it.

As I posted in comment 20, the Chrome bug for the CVE number Bob posted in comment 18 is here and is public:
  https://bugs.chromium.org/p/chromium/issues/detail?id=682020

Comment 25

[Andrew McCreight [:mccr8]]

Looking at the Chrome bug, Looben Yang says he submitted this to us as bug 1325511, which was fixed back to 52.

Bob, can you see if you can still reproduce this issue? It might be something else.

Comment 26

[Milan Sreckovic [:milan] (needinfo for best results)]

Non-withstanding the CVE numbers that are different, worth a try.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Sounds like we aren't sure if 53/54 are still affected here.

Comment 28

[Kelsey Gilbert [:jgilbert]]

Our previous fix from bug 1325511 is still in Beta54. I'll take a closer look.

Marking 55 as 'unaffected', 54 as 'affected' based on comment #17. 53 is still 'unknown'.

Comment 29

[Kelsey Gilbert [:jgilbert]]

I don't like their solution here, but I'm not going to fight upstream about it. Let's just take their fix.

Comment 30

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Bug-1328762-Cherry-pick-ANGLE-a4aaa2de57dc51243da35e.patch

Approval Request Comment
[Feature/Bug causing the regression]: upstream regression
[User impact if declined]: sec-crit
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Sure. POC repro is posted in the upstream bug.
[List of other uplifts needed for the feature/fix]: beta54, release53, esr52
[Is the change risky?]: no
[Why is the change risky/not risky?]: cherry-pick from upstream
[String changes made/needed]: none

Comment 31

[Bob Clary [:bc] (inactive)]

I wasn't able to reproduce any asan errors on Windows with a current nightly build. Beta is not available.

Comment 32

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8864318 [details] [diff] [review]
0001-Bug-1328762-Cherry-pick-ANGLE-a4aaa2de57dc51243da35e.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
no

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Yes, upstream bug is public and includes a repro case.

Which older supported branches are affected by this flaw?
beta54, release53, esr52

If not all supported branches, which bug introduced the flaw?
upstream regression

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
have backports

How likely is this patch to cause regressions; how much testing does it need?
unlikely

Comment 33

[Daniel Veditz [:dveditz]]

Comment on attachment 8864318 [details] [diff] [review]
0001-Bug-1328762-Cherry-pick-ANGLE-a4aaa2de57dc51243da35e.patch

Review of attachment 8864318 [details] [diff] [review]:
-----------------------------------------------------------------

sec-approval=dveditz

I'll let release drivers approve for release/ESR-52

Comment 34

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8864318 [details] [diff] [review]
0001-Bug-1328762-Cherry-pick-ANGLE-a4aaa2de57dc51243da35e.patch

OK, let's get this on release, beta, and esr52 today. This is the main driver for the 53.0.1 desktop release and for 52.1.1 esr. This can also make the 54 beta 5 build later tonight, I think.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/06bf49fb5795

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/d764f41b23b8

https://hg.mozilla.org/releases/mozilla-esr52/rev/bd4fcdee9a06 (FIREFOX_ESR_52_1_X_RELBRANCH - 52.1.1)
https://hg.mozilla.org/releases/mozilla-esr52/rev/238095c19891 (default - 52.2.0)

Comment 37

[Daniel Veditz [:dveditz]]

Since bug 1325511 didn't apply to ESR-45 I assume this one does not either.

Comment 38

[Julien Cristau [:jcristau] (back April 22)]

Is this bad enough we should try to release a new aurora54 build with this fix?

Comment 39

[Andrei Vaida [:avaida]]

We were unable to reproduce the crash on Windows 7, Windows 10 and Ubuntu 16.04. We tried replicating it using both the instructions from Comment 0 and the test case from Comment 20, with no success.

We cannot confirm if this bug is fixed on 52.1.1esr-build1 or 53.0.2-build1.

Comment 40

[Bob Clary [:bc] (inactive)]

I would just to point out that Looben Yang reported the issue to Chrome on Jan 17 while this bug was reported on Jan 4.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

(In reply to Julien Cristau [:jcristau] from comment #38)
> Is this bad enough we should try to release a new aurora54 build with this
> fix?

Comment 42

[Liz Henry (:lizzard) (relman/hg->git project)]

From talking with Dan and Jeff the other day, no, as the vulnerability reported was partially fixed already and what was left wasn't sec-critical.

Comment 43

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #41)
> (In reply to Julien Cristau [:jcristau] from comment #38)
> > Is this bad enough we should try to release a new aurora54 build with this
> > fix?

We discussed this at the Dawn project coordination meeting today. Like Liz said the severity doesn't warrant a chemspill situation.

Comment 44

[Daniel Veditz [:dveditz]]

(In reply to Bob Clary [:bc:] from comment #40)
> I would just to point out that Looben Yang reported the issue to Chrome on
> Jan 17 while this bug was reported on Jan 4.

He was really reporting bug 1325511 (from December) to Chrome: see bug 1325511 comment 15, 18. Their resulting fix covered this case too whereas ours did not.

Comment 45

[Bob Clary [:bc] (inactive)]

Attachment: crash report + log

During a retest of urls which have crashed in Bughunter since July 1, this crash was reproduced on Windows 7 32bit with build rv:56.0 20170719173022 on url:

https://sketchfab.com/models/67b90d3c88e244dc90917f46ef7ff9c5

exploitable rated this as high which fits with Crash address: 0xffffffffe5e5e621 and eax = 0xe5e5e5e5.

I would say this is the same as the original crash and this *isn't* fixed.

This is the only example I've seen so far but the retest is still ongoing.

Comment 46

[Bob Clary [:bc] (inactive)]

Per conversation with dveditz, I bug 1382851 to track this.