summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoe Downing <joedow@google.com>2020-10-16 09:15:21 -0700
committerMichael BrĂ¼ning <michael.bruning@qt.io>2020-12-07 22:24:43 +0000
commit275dca60b7074c0edc4d6f826f01304b783337dc (patch)
treea9def711f9733dc11fc1b40f77a019e4b252de44
parent05386001f90b9b2414b0feb8a2b2cf592246213e (diff)
[Backport] CVE-2020-16028: Heap buffer overflow in WebRTC
Manual backport of patch originally reviewed on https://webrtc-review.googlesource.com/c/src/+/188900: Fixing ASAN container-overflow error in DxgiOutputDuplicator The DxgiOutputDuplicator uses a vector<byte> to hold the rects that have changed on the screen. It then iterates over the vector to grab each rect and apply it to the updated_region. There is vector resizing logic which checks the 'capacity' of the vector and determines whether there is enough space for the changed rect data. Often the 'capacity' and 'size' of the vector are equal but that isn't always true. When the capacity is greater than size, and the number of changed rects is high enough, rect data will be written past the element pointed to by (data() + size()) and this is the error that ASAN is warning of. The fix is to use size() instead of capacity() when determining whether to resize the vector and as the buffer size we provide to the Windows API. There are no other usages of this vector so there aren't any problems caused by the size/capacity discrepancy in the existing builds. The ASAN issue is worth fixing in case someone comes along and decides to use the vector differently (e.g rely on the size instead of capacity so some of the rects are not counted). Bug: chromium:1138446 Change-Id: I3041091423de889e0f8aabc56ece9466a3000b4f Reviewed-by: Jamie Walch <jamiewalch@chromium.org> Commit-Queue: Joe Downing <joedow@google.com> Cr-Commit-Position: refs/heads/master@{#32425} Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r--chromium/third_party/webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc7
1 files changed, 3 insertions, 4 deletions
diff --git a/chromium/third_party/webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc b/chromium/third_party/webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc
index d25b28e22fb..ad965a61b83 100644
--- a/chromium/third_party/webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc
+++ b/chromium/third_party/webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc
@@ -274,7 +274,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
return false;
}
- if (metadata_.capacity() < frame_info.TotalMetadataBufferSize) {
+ if (metadata_.size() < frame_info.TotalMetadataBufferSize) {
metadata_.clear(); // Avoid data copy
metadata_.resize(frame_info.TotalMetadataBufferSize);
}
@@ -284,7 +284,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
reinterpret_cast<DXGI_OUTDUPL_MOVE_RECT*>(metadata_.data());
size_t move_rects_count = 0;
_com_error error = duplication_->GetFrameMoveRects(
- static_cast<UINT>(metadata_.capacity()), move_rects, &buff_size);
+ static_cast<UINT>(metadata_.size()), move_rects, &buff_size);
if (error.Error() != S_OK) {
RTC_LOG(LS_ERROR) << "Failed to get move rectangles, error "
<< error.ErrorMessage() << ", code " << error.Error();
@@ -295,8 +295,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
RECT* dirty_rects = reinterpret_cast<RECT*>(metadata_.data() + buff_size);
size_t dirty_rects_count = 0;
error = duplication_->GetFrameDirtyRects(
- static_cast<UINT>(metadata_.capacity()) - buff_size, dirty_rects,
- &buff_size);
+ static_cast<UINT>(metadata_.size()) - buff_size, dirty_rects, &buff_size);
if (error.Error() != S_OK) {
RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles, error "
<< error.ErrorMessage() << ", code " << error.Error();