diff options
author | Ted Meyer <tmathmeyer@chromium.org> | 2022-02-14 21:00:35 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2022-05-03 20:14:37 +0000 |
commit | fb0edb35aea02b69ce9deb3c3a25abc816c3c313 (patch) | |
tree | 8e23c91362c6608e8e19ffe8ee49d06edceadd46 | |
parent | 6156c93cb921ab543a1450840581adeb1e2db3b3 (diff) | |
download | qtwebengine-chromium-fb0edb35aea02b69ce9deb3c3a25abc816c3c313.tar.gz |
[Backport] CVE-2022-0796: Use after free in Media
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3451494:
Guard BatchingMediaLog::event_handlers_ with lock
It seems that despite MediaLog::OnWebMediaPlayerDestroyed and
MediaLog::AddLogRecord both grabbing a lock,
BatchingMediaLog::AddLogRecordLocked can escape the lock handle by
posting BatchingMediaLog::SendQueuedMediaEvents, causing a race.
When the addition of an event is interrupted by the deletion of a player
due to player culling in MediaInspectorContextImpl, a UAF can occur.
R=dalecurtis
Bug: 1295786
Change-Id: I77df94988f806e4d98924669d27860e50455299d
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#970815}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/content/renderer/media/batching_media_log.cc | 37 | ||||
-rw-r--r-- | chromium/content/renderer/media/batching_media_log.h | 20 |
2 files changed, 28 insertions, 29 deletions
diff --git a/chromium/content/renderer/media/batching_media_log.cc b/chromium/content/renderer/media/batching_media_log.cc index 3023fb14297..eb116e86725 100644 --- a/chromium/content/renderer/media/batching_media_log.cc +++ b/chromium/content/renderer/media/batching_media_log.cc @@ -55,9 +55,9 @@ BatchingMediaLog::BatchingMediaLog( scoped_refptr<base::SingleThreadTaskRunner> task_runner, std::vector<std::unique_ptr<EventHandler>> event_handlers) : task_runner_(std::move(task_runner)), - event_handlers_(std::move(event_handlers)), tick_clock_(base::DefaultTickClock::GetInstance()), last_ipc_send_time_(tick_clock_->NowTicks()), + event_handlers_(std::move(event_handlers)), ipc_send_pending_(false), logged_rate_limit_warning_(false) { DCHECK(RenderThread::Get()) @@ -81,6 +81,7 @@ BatchingMediaLog::~BatchingMediaLog() { } void BatchingMediaLog::OnWebMediaPlayerDestroyedLocked() { + base::AutoLock lock(lock_); for (const auto& handler : event_handlers_) handler->OnWebMediaPlayerDestroyed(); } @@ -203,32 +204,30 @@ std::string BatchingMediaLog::MediaEventToMessageString( void BatchingMediaLog::SendQueuedMediaEvents() { DCHECK(task_runner_->BelongsToCurrentThread()); + base::AutoLock auto_lock(lock_); - std::vector<media::MediaLogRecord> events_to_send; - { - base::AutoLock auto_lock(lock_); - DCHECK(ipc_send_pending_); - ipc_send_pending_ = false; - - if (last_duration_changed_event_) { - queued_media_events_.push_back(*last_duration_changed_event_); - last_duration_changed_event_.reset(); - } + DCHECK(ipc_send_pending_); + ipc_send_pending_ = false; - if (last_buffering_state_event_) { - queued_media_events_.push_back(*last_buffering_state_event_); - last_buffering_state_event_.reset(); - } + if (last_duration_changed_event_) { + queued_media_events_.push_back(*last_duration_changed_event_); + last_duration_changed_event_.reset(); + } - queued_media_events_.swap(events_to_send); - last_ipc_send_time_ = tick_clock_->NowTicks(); + if (last_buffering_state_event_) { + queued_media_events_.push_back(*last_buffering_state_event_); + last_buffering_state_event_.reset(); } - if (events_to_send.empty()) + last_ipc_send_time_ = tick_clock_->NowTicks(); + + if (queued_media_events_.empty()) return; for (const auto& handler : event_handlers_) - handler->SendQueuedMediaEvents(events_to_send); + handler->SendQueuedMediaEvents(queued_media_events_); + + queued_media_events_.clear(); } void BatchingMediaLog::SetTickClockForTesting( diff --git a/chromium/content/renderer/media/batching_media_log.h b/chromium/content/renderer/media/batching_media_log.h index 91a505326fc..b75593555aa 100644 --- a/chromium/content/renderer/media/batching_media_log.h +++ b/chromium/content/renderer/media/batching_media_log.h @@ -63,9 +63,6 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog { scoped_refptr<base::SingleThreadTaskRunner> task_runner_; - // impl for sending queued events. - std::vector<std::unique_ptr<EventHandler>> event_handlers_; - // |lock_| protects access to all of the following member variables. It // allows any render process thread to AddEvent(), while preserving their // sequence for throttled send on |task_runner_| and coherent retrieval by @@ -73,19 +70,22 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog { // guarantees provided by MediaLog, since SendQueuedMediaEvents must also // be synchronized with respect to AddEvent. mutable base::Lock lock_; - const base::TickClock* tick_clock_; - base::TimeTicks last_ipc_send_time_; - std::vector<media::MediaLogRecord> queued_media_events_; + const base::TickClock* tick_clock_ GUARDED_BY(lock_); + base::TimeTicks last_ipc_send_time_ GUARDED_BY(lock_); + std::vector<media::MediaLogRecord> queued_media_events_ GUARDED_BY(lock_); + + // impl for sending queued events. + std::vector<std::unique_ptr<EventHandler>> event_handlers_ GUARDED_BY(lock_); // For enforcing max 1 pending send. - bool ipc_send_pending_; + bool ipc_send_pending_ GUARDED_BY(lock_); // True if we've logged a warning message about exceeding rate limits. - bool logged_rate_limit_warning_; + bool logged_rate_limit_warning_ GUARDED_BY(lock_); // Limits the number of events we send over IPC to one. - base::Optional<media::MediaLogRecord> last_duration_changed_event_; - base::Optional<media::MediaLogRecord> last_buffering_state_event_; + base::Optional<media::MediaLogRecord> last_duration_changed_event_ GUARDED_BY(lock_); + base::Optional<media::MediaLogRecord> last_buffering_state_event_ GUARDED_BY(lock_); // Holds the earliest MEDIA_ERROR_LOG_ENTRY event added to this log. This is // most likely to contain the most specific information available describing |