Skip to content

Commit dc8f447

Browse files
joedow-42Commit Bot
authored andcommitted
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). (cherry picked from commit ea3e321) Bug: chromium:1138446 Change-Id: I3041091423de889e0f8aabc56ece9466a3000b4f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188900 Reviewed-by: Jamie Walch <[email protected]> Commit-Queue: Joe Downing <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#32425} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190165 Commit-Queue: Jamie Walch <[email protected]> Cr-Commit-Position: refs/branch-heads/4280@{#10} Cr-Branched-From: 75b9ab6-refs/heads/master@{#32272}
1 parent 0704197 commit dc8f447

File tree

1 file changed

+3
-4
lines changed

1 file changed

+3
-4
lines changed

modules/desktop_capture/win/dxgi_output_duplicator.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
273273
return false;
274274
}
275275

276-
if (metadata_.capacity() < frame_info.TotalMetadataBufferSize) {
276+
if (metadata_.size() < frame_info.TotalMetadataBufferSize) {
277277
metadata_.clear(); // Avoid data copy
278278
metadata_.resize(frame_info.TotalMetadataBufferSize);
279279
}
@@ -283,7 +283,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
283283
reinterpret_cast<DXGI_OUTDUPL_MOVE_RECT*>(metadata_.data());
284284
size_t move_rects_count = 0;
285285
_com_error error = duplication_->GetFrameMoveRects(
286-
static_cast<UINT>(metadata_.capacity()), move_rects, &buff_size);
286+
static_cast<UINT>(metadata_.size()), move_rects, &buff_size);
287287
if (error.Error() != S_OK) {
288288
RTC_LOG(LS_ERROR) << "Failed to get move rectangles, error "
289289
<< error.ErrorMessage() << ", code " << error.Error();
@@ -294,8 +294,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
294294
RECT* dirty_rects = reinterpret_cast<RECT*>(metadata_.data() + buff_size);
295295
size_t dirty_rects_count = 0;
296296
error = duplication_->GetFrameDirtyRects(
297-
static_cast<UINT>(metadata_.capacity()) - buff_size, dirty_rects,
298-
&buff_size);
297+
static_cast<UINT>(metadata_.size()) - buff_size, dirty_rects, &buff_size);
299298
if (error.Error() != S_OK) {
300299
RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles, error "
301300
<< error.ErrorMessage() << ", code " << error.Error();

0 commit comments

Comments
 (0)