capn, this looks like a bug in swiftshader. PTAL.

Is this something we ship to users?

It's in M59 on Windows and Linux, so not yet shipping to users, but soon.

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/f34d1ace76a7e384685ebc5395141295cf1c618f

commit f34d1ace76a7e384685ebc5395141295cf1c618f
Author: Nicolas Capens <capn@google.com>
Date: Mon May 08 21:12:42 2017

Fix buffer overflow.

 Bug chromium:719291 

Change-Id: I5ddf6d45d3a66a4b626ec1d73995a2a4fd4b28b9
Reviewed-on: https://swiftshader-review.googlesource.com/9668
Reviewed-by: Nicolas Capens <capn@google.com>
Tested-by: Nicolas Capens <capn@google.com>

[modify] https://crrev.com/f34d1ace76a7e384685ebc5395141295cf1c618f/src/Reactor/SubzeroReactor.cpp

ochang@, can you give me an indication of whether this will have to be merged into M59?

I'm not too worried about it from a software correctness point of view. SwiftShader is passing an extensive conformance test suite, so this is only triggered by an edge case which is probably not going to affect many users. So from that perspective I'm fine with only having the fix available in the next release.

From a security perspective, someone could use it to read a part of the GPU process's stack content, from Javascript, iff the GPU is blacklisted. Most likely it's just "random" data from another call withing SwiftShader, and not useful in any way for any malicious intent.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/915256dfeb6b6f87c5b2d145c08fecfcf247553b

commit 915256dfeb6b6f87c5b2d145c08fecfcf247553b
Author: capn <capn@chromium.org>
Date: Tue May 09 15:16:29 2017

Roll SwiftShader 5f72693..f34d1ac

https://swiftshader.googlesource.com/SwiftShader.git/+log/5f72693..f34d1ac

Mainly fixes a buffer overrun.

BUG= 719291 

TBR=kbr@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2866083003
Cr-Commit-Position: refs/heads/master@{#470338}

[modify] https://crrev.com/915256dfeb6b6f87c5b2d145c08fecfcf247553b/DEPS

Thanks for the fix! 

"random" data on the stack can aid exploitation (e.g. bypassing ASLR), especially if it can be read via javascript :) Please do merge this to M59 if possible.

btw, we have another open bug: https://bugs.chromium.org/p/chromium/issues/detail?id=719720. Can you please take a look to see if it's a dupe? Thanks!

capn, btw, was there a security review for swiftshader? do we have any direct fuzzing for this?

We've hit swiftshader before through fuzzing Android a while back, and it was extremely crashy. Given that it's going to be running in a relatively privileged (GPU) process, we should make sure it's in a good state before shipping it to users...

ClusterFuzz has detected this issue as fixed in range 470314:470339.

Detailed report: https://clusterfuzz.com/testcase?key=5404955224834048

Fuzzer: inferno_twister_c
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Stack-buffer-overflow READ {*}
Crash Address: 0x053940ef
Crash State:
  sw::Nucleus::createConstantVector
  sw::Nucleus::createNot
  (class
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=464127:464479
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=470314:470339

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5404955224834048


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5404955224834048 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Thanks for clarifying the security concern. I'll try to get it merged into M59.

Regarding security review, SwiftShader has already been shipping to users for years as a separately downloaded component. In M59 we've integrated it into the Chromium build and ship it with the installer. So from a security point of view not much has changed. We're just getting a lot more visibility into it due to having ASAN builds and fuzzing.

To further put this into perspective, we get GPU driver crash reports all the time, and we're blind to their less fatal issues. So these drivers are generally untrusted, and the GPU team is vigilant about exploits. In light of this and the general complexity of graphics APIs I think SwiftShader is doing exceptionally well.

That said I'd be happy to have a discussion about how we can improve it to the highest standards in the most productive way. Note that we're quite understaffed though and support many other projects big and small, so I can only do so much.

This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

 awhalley@, is this merge good to take in for M59?

govind@ - yep, good for 59

Approving merge to M59 branch 3071 based on comment #20. Please merge before 5:00 PM PT today if possible so we can pick it up for this week beta release. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b01b9acf5b1e54795099cdd7a62dc07ce01f06e7

commit b01b9acf5b1e54795099cdd7a62dc07ce01f06e7
Author: capn <capn@chromium.org>
Date: Mon May 15 21:48:57 2017

Roll SwiftShader 30385f0..9ed48ba

https://swiftshader.googlesource.com/SwiftShader.git/+log/30385f0..9ed48ba

- Fixes LTO causing hard crash on Linux official builds.
- Fixes buffer overflow.

BUG= 720933 
BUG= 719291 

NOTRY=true
NOPRESUBMIT=true

TBR=kbr@chromium.org

Review-Url: https://codereview.chromium.org/2888433002
Cr-Commit-Position: refs/branch-heads/3071@{#568}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/b01b9acf5b1e54795099cdd7a62dc07ce01f06e7/DEPS

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/4c54802b8c32deb32093688a836a8c17cdfe8bc6

commit 4c54802b8c32deb32093688a836a8c17cdfe8bc6
Author: Nicolas Capens <capn@google.com>
Date: Thu May 18 15:31:13 2017

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot