Issue metadata
Sign in to add a comment
|
Security: Incorrect validation of CopyBufferSubData in ANGLE
Reported by
tjbecker...@gmail.com,
Oct 31 2016
|
||||||||||||||||||||||
Issue description
VULNERABILITY DETAILS
Please provide a brief explanation of the security issue.
In src/third_party/angle/src/libGLESv2/entry_points_gles_3_0.cpp, in the
function CopyBufferSubData, there is incorrect validation of the
readOffset and writeOffset values:
if (readOffset < 0 || writeOffset < 0 || size < 0 ||
static_cast<unsigned int>(readOffset + size) > readBuffer->getSize() ||
static_cast<unsigned int>(writeOffset + size) > writeBuffer->getSize())
{
context->handleError(Error(GL_INVALID_VALUE));
return;
}
readOffset and writeOffset are both type GLintptr, which is 64 bits long.
static_cast<unsigned int>(readOffset + size) can result in truncation if
readOffset + size exceeds 32 bits, causing the validation to pass
with invalid offsets.
This is the only validation of these arguments that occurs on the GPU process.
Note that this is only reachable via ES3, but this is now enabled by default
on Windows: https://codereview.chromium.org/2354423002
VERSION
Chrome Version: 55.0.2873.0 + beta
Operating System: Windows
REPRODUCTION CASE
A demonstration of the incorrect validation is attached as poc.cc.
,
Nov 1 2016
,
Nov 1 2016
,
Nov 1 2016
jmadill: can you take this? FWIW: chrome's command buffer validation handles overflow checking, so it won't reach ANGLE. That said, it would be nice for ANGLE to beef up.
,
Nov 1 2016
Looking.
,
Nov 2 2016
zmo@, could point me at where this validation happens in the command buffer?
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/d2f0c74c8517365302fffcc2dc7b86778c2a6211 commit d2f0c74c8517365302fffcc2dc7b86778c2a6211 Author: Jamie Madill <jmadill@chromium.org> Date: Wed Nov 02 14:34:41 2016 Use safe math in ValidateCopyBufferSubData. This should fix any potential out of bounds reads/writes. BUG= chromium:660854 Change-Id: Iffa00e4551d7362115cbf023a09b1d0e15f724c8 Reviewed-on: https://chromium-review.googlesource.com/405816 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> [modify] https://crrev.com/d2f0c74c8517365302fffcc2dc7b86778c2a6211/src/libANGLE/validationES3.cpp [modify] https://crrev.com/d2f0c74c8517365302fffcc2dc7b86778c2a6211/src/tests/gl_tests/BufferDataTest.cpp
,
Nov 3 2016
Nice find! Thanks for the report.
,
Nov 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b42220eced72b0a5333ba28c6dd7ccf3a5178d40 commit b42220eced72b0a5333ba28c6dd7ccf3a5178d40 Author: ynovikov <ynovikov@chromium.org> Date: Fri Nov 04 19:27:25 2016 Roll ANGLE eb66a6e..bbe9fb5 https://chromium.googlesource.com/angle/angle.git/+log/eb66a6e..bbe9fb5 BUG=None,chromium:568170,chromium:660854,chromium:661592 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/2479673003 Cr-Commit-Position: refs/heads/master@{#429975} [modify] https://crrev.com/b42220eced72b0a5333ba28c6dd7ccf3a5178d40/DEPS
,
Nov 11 2016
,
Nov 11 2016
,
Nov 13 2016
,
Nov 13 2016
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.
,
Nov 13 2016
I don't think this is necessary to merge. As zmo says in #4, command buffer should filter invalid GL calls from reaching ANGLE. Mo can you confirm?
,
Nov 14 2016
+awhalley@ (Security TPM).
,
Nov 14 2016
zmo@, could you please reply to comment #14. Thank you.
,
Nov 14 2016
Yes, I agree with jmadill. We don't have a security issue in Chrome as far as this bug is concerned, so no need to merge.
,
Nov 14 2016
Also, FYI, merging this change should not be terribly difficult. It may not have a dependency, would have to check, but I don't think so.
,
Nov 14 2016
If merge to M55 is NOT is needed, then please remove "Merge-Review-55" label. Thank you.
,
Nov 14 2016
,
Nov 18 2016
No reward for this one as it can't be triggered from Chrome, per comment 17
,
Nov 18 2016
That's too bad - this bug would become triggered from Chrome once we complete issue 602688 and ship it on Windows. It would be unfortunate to discourage people from looking at bugs in ANGLE's validation that will become serious security issues in the future.
,
Nov 18 2016
From issue 602688 "while still performing security checks" :-) Thanks for the heads up - I'll send this back to the panel to reconsider given that information.
,
Nov 29 2016
,
Nov 29 2016
,
Nov 29 2016
On second thoughts, the panel decided to reward $1,000 for this bug, thanks!
,
Nov 29 2016
Awesome!
,
Nov 29 2016
That's amazing! Thank you!
,
Dec 2 2016
,
Dec 2 2016
,
Jan 4 2017
,
Feb 17 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
,
Apr 25 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ta...@google.com
, Oct 31 2016Labels: Security_Severity-Medium Security_Impact-Stable OS-All
Owner: zmo@chromium.org
Status: Assigned (was: Unconfirmed)