New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 595836 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security

Blocking:
issue 595834



Sign in to add a comment

libANGLE buffer-overflow (part of pwn2own exploit)

Project Member Reported by infe...@chromium.org, Mar 17 2016

Issue description

// src/third_party/angle/src/libANGLE/Program.cpp
template <typename DestT>
void Program::getUniformInternal(GLint location, DestT *dataOut) const
{
 const VariableLocation &locationInfo = mData.mUniformLocations[location];
 const LinkedUniform &uniform = mData.mUniforms[locationInfo.index];
 const uint8_t *srcPointer =
uniform.getDataPtrToElement(locationInfo.element);
 GLenum componentType = VariableComponentType(uniform.type);
 if (componentType == GLTypeToGLenum<DestT>::value)
 {
 memcpy(dataOut, srcPointer, uniform.getElementSize());
 return;
 }
 int components = VariableComponentCount(uniform.type) *
uniform.elementCount();
 switch (componentType)
 {
 case GL_INT:
 UniformStateQueryCastLoop<GLint>(dataOut, srcPointer, components);
 break;
 case GL_UNSIGNED_INT:
 UniformStateQueryCastLoop<GLuint>(dataOut, srcPointer, components);
 break;
 case GL_BOOL:
 UniformStateQueryCastLoop<GLboolean>(dataOut, srcPointer,
components);
 break;
 case GL_FLOAT:
 UniformStateQueryCastLoop<GLfloat>(dataOut, srcPointer, components);
 break;
 default:
 UNREACHABLE();
 }
}
Chromium's OpenGL rendering preprocess is handled by GPU process's libANGLE. Above
function, getUniformInternal is a function that handles glGetUniform family. This function
is used when retrieving the value of the shader code's variable.
In OpenGL API documentation, glGetUniform family does not process arrays in bulk but
rather do it individually. However, when the type of src and the type of dest are different, the
following code gets executed:
int components = VariableComponentCount(uniform.type) * uniform.elementCount();
As seen above, by multiplying elementCount , the whole array gets processed at once. So
whenever the types of src and dest do not match, a buffer overflow occurs.
For example, the following code results in a buffer overflow:
Shader:
 ivec2 tmp[100];
Code:
 glGetUniformfv(tmp_location, vals);
However, the following code does not result in a buffer overflow:
Shader:
 ivec2 tmp[100];
Code:
 glGetUniformiv(tmp_location, vals);
Take a look at the following function:
error::Error GLES2DecoderImpl::HandleGetUniformfv(uint32 immediate_data_size,
 const void* cmd_data) {
 const gles2::cmds::GetUniformfv& c =
 *static_cast<const gles2::cmds::GetUniformfv*>(cmd_data);
 GLuint program = c.program;
 GLint fake_location = c.location;
 GLuint service_id;
 GLint real_location = -1;
 Error error;
 typedef cmds::GetUniformfv::Result Result;
 Result* result;
 GLenum result_type;
 GLsizei result_size;
 if (GetUniformSetup(program, fake_location, c.params_shm_id,
 c.params_shm_offset, &error, &real_location, &service_id,
 reinterpret_cast<void**>(&result), &result_type,
 &result_size)) {
 if (result_type == GL_BOOL || result_type == GL_BOOL_VEC2 ||
 result_type == GL_BOOL_VEC3 || result_type == GL_BOOL_VEC4) {
 GLsizei num_values = result_size / sizeof(Result::Type);
 scoped_ptr<GLint[]> temp(new GLint[num_values]);
 glGetUniformiv(service_id, real_location, temp.get()); <<< overflow 1
 GLfloat* dst = result->GetData();
 for (GLsizei ii = 0; ii < num_values; ++ii) {
 dst[ii] = (temp[ii] != 0);
 }
 } else {
 glGetUniformfv(service_id, real_location, result->GetData()); <<<
overflow 2
 }
 }
 return error;
}
There are two ways to trigger the vulnerability. The first method is to use glGetUniformfv and
bvec to perform heap overflow, and the second method is to overflow the shared memory like
last year. The first method is difficult to exploit since the heap metadata is written as well. So we
used the same way to exploit as last year.
 
Labels: Pri-0
Labels: -Pri-0 Security_Severity-High Security_Impact-Stable Pri-1

Comment 3 by wfh@chromium.org, Mar 17 2016

the reporter just reminded us that this is in the same piece of code to last year's pwn2own bug.
ZDI-CAN-3623
Cc: bajones@chromium.org vmi...@chromium.org enne@chromium.org
Please suggest angle expert to own this soon.
Cc: sshruthi@chromium.org

Comment 7 by sshru...@google.com, Mar 17 2016

Cc: shannonwoods@chromium.org cwallez@chromium.org geoffl...@chromium.org jmad...@chromium.org
cwallez@, jmadill@, geofflang@, can one of you guys take a look?

Comment 8 by sshru...@google.com, Mar 17 2016

Owner: cwallez@chromium.org
Corentin is looking into it.
The patch is up for review and in a CQ dry run. https://chromium-review.googlesource.com/#/c/333771/

When it is landed, do we need to merge the fix in some Chrome branches?
Yes for M-40 we would need to merge.
You need to create a separate angle branch for both stable and beta branches. and then you need to modify this DEPS file for these (make sure to change in this location).  E.g. stable one at  https://chromegw.corp.google.com/viewvc/chrome-internal/trunk/tools/buildspec/branches/2623/DEPS?view=markup. (see major revision from omahaproxy).
Project Member

Comment 12 by sheriffbot@chromium.org, Mar 18 2016

Labels: M-49
Cc: gov...@chromium.org
Labels: Merge-Request-50
Merge Request for M-50 

(Merge request for M-49 will follow after approval of M-50)
Labels: Merge-Request-49
Requesting M-49  - want to get this into M49 by 5PM todau.
Labels: -Merge-Request-49 Merge-Approved-49
Merge approved for M49 (branch 2623)
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/3ea54ba890f25fcce9213d8ab7c7f8a9fdf10cad

commit 3ea54ba890f25fcce9213d8ab7c7f8a9fdf10cad
Author: Corentin Wallez <cwallez@chromium.org>
Date: Thu Mar 17 21:26:58 2016

Program::getUniformInternal: return only one array element

When getUniformInternal detected a mismatch between the glGetUniform
type and the uniform type, it entered a code path where it wrongly wrote
the whole array instead of a single element, causing a buffer overflow.

Adds a regression test.

BUG= 595836 

Change-Id: Id7372faece276d28363a30bf3183497d97357c76
Reviewed-on: https://chromium-review.googlesource.com/333771
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/3ea54ba890f25fcce9213d8ab7c7f8a9fdf10cad/src/tests/gl_tests/UniformTest.cpp
[modify] https://crrev.com/3ea54ba890f25fcce9213d8ab7c7f8a9fdf10cad/src/libANGLE/Program.cpp

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 22 2016

Labels: -merge-approved-49 merge-merged-2623
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/3e7ae4249217eb66dfad09419525066bcbdb560b

commit 3e7ae4249217eb66dfad09419525066bcbdb560b
Author: Corentin Wallez <cwallez@chromium.org>
Date: Thu Mar 17 21:26:58 2016

Program::getUniformInternal: return only one array element

When getUniformInternal detected a mismatch between the glGetUniform
type and the uniform type, it entered a code path where it wrongly wrote
the whole array instead of a single element, causing a buffer overflow.

Adds a regression test.

BUG= 595836 

Change-Id: Id7372faece276d28363a30bf3183497d97357c76
Reviewed-on: https://chromium-review.googlesource.com/333771
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/334394
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/3e7ae4249217eb66dfad09419525066bcbdb560b/src/tests/gl_tests/UniformTest.cpp
[modify] https://crrev.com/3e7ae4249217eb66dfad09419525066bcbdb560b/src/libANGLE/Program.cpp

Comment 19 by tin...@google.com, Mar 22 2016

Labels: -Merge-Request-50 Merge-Approved-50
Merge approved for M50 (branch 2661). Pls go ahead merge.
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 22 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/22de2f04232012afa2dd23bf40f66f4a3bce9133

commit 22de2f04232012afa2dd23bf40f66f4a3bce9133
Author: Corentin Wallez <cwallez@chromium.org>
Date: Thu Mar 17 21:26:58 2016

Program::getUniformInternal: return only one array element

When getUniformInternal detected a mismatch between the glGetUniform
type and the uniform type, it entered a code path where it wrongly wrote
the whole array instead of a single element, causing a buffer overflow.

Adds a regression test.

BUG= 595836 

Change-Id: Id7372faece276d28363a30bf3183497d97357c76
Reviewed-on: https://chromium-review.googlesource.com/333771
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/334288
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/22de2f04232012afa2dd23bf40f66f4a3bce9133/src/tests/gl_tests/UniformTest.cpp
[modify] https://crrev.com/22de2f04232012afa2dd23bf40f66f4a3bce9133/src/libANGLE/Program.cpp

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=85614

------------------------------------------------------------------
r85614 | kbr@google.com | 2016-03-22T22:48:18.972026Z

-----------------------------------------------------------------
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=85615

------------------------------------------------------------------
r85615 | kbr@google.com | 2016-03-22T22:54:46.711271Z

-----------------------------------------------------------------
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/4e9536d0f612dab15dd6722975155b78b92ca0ae

commit 4e9536d0f612dab15dd6722975155b78b92ca0ae
Author: Corentin Wallez <cwallez@chromium.org>
Date: Tue Mar 22 23:37:13 2016

Revert tests changes of 3e7ae4249217eb66dfad09419525066bcbdb560b

The test changes had a function pointer type mismatch on some platforms,
causing build failures. Revert the tests changes and keep only the fix
for the buffer overflow.

BUG= 595836 

Change-Id: Ia2db42dc7b19fc46676addaf8ad36f29d9bf69e5
Reviewed-on: https://chromium-review.googlesource.com/334396
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/4e9536d0f612dab15dd6722975155b78b92ca0ae/src/tests/gl_tests/UniformTest.cpp

Project Member

Comment 24 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/c46018b8598dad46f6834411f27e12e90bb62a56

commit c46018b8598dad46f6834411f27e12e90bb62a56
Author: Corentin Wallez <cwallez@chromium.org>
Date: Tue Mar 22 23:37:13 2016

Revert tests changes of 22de2f04232012afa2dd23bf40f66f4a3bce9133

The test changes had a function pointer type mismatch on some platforms,
causing build failures. Revert the tests changes and keep only the fix
for the buffer overflow.

BUG= 595836 

Change-Id: Ia2db42dc7b19fc46676addaf8ad36f29d9bf69e5
Reviewed-on: https://chromium-review.googlesource.com/334398
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/c46018b8598dad46f6834411f27e12e90bb62a56/src/tests/gl_tests/UniformTest.cpp

Project Member

Comment 25 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=85617

------------------------------------------------------------------
r85617 | kbr@google.com | 2016-03-22T23:48:11.781001Z

-----------------------------------------------------------------
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 22 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=85618

------------------------------------------------------------------
r85618 | kbr@google.com | 2016-03-22T23:48:36.601813Z

-----------------------------------------------------------------
Project Member

Comment 27 by ClusterFuzz, Mar 23 2016

Status: Fixed (was: Assigned)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges.

- Your friendly ClusterFuzz
Project Member

Comment 28 by ClusterFuzz, Mar 23 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Release-2-M49 reward-ineligible
Labels: CVE-2016-1649
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/6596c465a7817723f9093ece444892b2817a3221

commit 6596c465a7817723f9093ece444892b2817a3221
Author: Corentin Wallez <cwallez@chromium.org>
Date: Thu Mar 17 21:26:58 2016

Program::getUniformInternal: return only one array element

Reland with a compilation fix for GPU Builder, with a fix for
UniformTest.BooleanArrayUniformStateQuery and better formatting.

When getUniformInternal detected a mismatch between the glGetUniform
type and the uniform type, it entered a code path where it wrongly wrote
the whole array instead of a single element, causing a buffer overflow.

Adds a regression test.

BUG= 595836 

Change-Id: Ie860b87ad56046483650f457457116cc22bf3c0f
Reviewed-on: https://chromium-review.googlesource.com/334448
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/6596c465a7817723f9093ece444892b2817a3221/src/tests/gl_tests/UniformTest.cpp
[modify] https://crrev.com/6596c465a7817723f9093ece444892b2817a3221/src/libANGLE/Program.cpp

Project Member

Comment 32 by bugdroid1@chromium.org, Mar 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/7e2ba9eee3bfeb50d7447038a4b372456de63528

commit 7e2ba9eee3bfeb50d7447038a4b372456de63528
Author: Jamie Madill <jmadill@chromium.org>
Date: Tue Mar 29 19:22:01 2016

Fix warning introduced in "Program::getUniformInternal: return only one array element"

The warning does not seem to occur on the Chromium bots, but shows
when compiling ANGLE standalone.

BUG= 595836 

Change-Id: I3c22bbea263223f9e92f82229817e9e9894a46ad
Reviewed-on: https://chromium-review.googlesource.com/335576
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/7e2ba9eee3bfeb50d7447038a4b372456de63528/src/tests/gl_tests/UniformTest.cpp

Project Member

Comment 33 by bugdroid1@chromium.org, Apr 2 2016

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

commit 6a5b73fc8615892260d6f47242d1d6a642e3dcfc
Author: jmadill <jmadill@chromium.org>
Date: Sat Apr 02 01:36:02 2016

Roll ANGLE 00f394e..0c0d800

https://chromium.googlesource.com/angle/angle.git/+log/00f394e..0c0d800

BUG=593448, 598944 , 595836 ,590870
TBR=geofflang@chromium.org
TEST=bots

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

Review URL: https://codereview.chromium.org/1849013003

Cr-Commit-Position: refs/heads/master@{#384777}

[modify] https://crrev.com/6a5b73fc8615892260d6f47242d1d6a642e3dcfc/DEPS

Project Member

Comment 34 by sheriffbot@chromium.org, Jun 29 2016

Labels: -Restrict-View-SecurityNotify
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
Project Member

Comment 35 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 36 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment