admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing ASAN container-overflow error in DxgiOutputDuplicator

Browse Source 
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-on: https://webrtc-review.googlesource.com/c/src/+/188900
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@google.com>
Cr-Commit-Position: refs/heads/master@{#32425}
This commit is contained in:
Joe Downing 2020-10-16 09:15:21 -07:00 committed by Commit Bot
parent 88329b9266
commit ea3e3215e0
1 changed files with 3 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
modules/desktop_capture/win/dxgi_output_duplicator.cc
View File

@ -271,7 +271,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);
}
@ -281,7 +281,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: "
<< desktop_capture::utils::ComErrorToString(error);
@ -292,8 +292,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: "
<< desktop_capture::utils::ComErrorToString(error);