From c16b1652b5fb848c20bc2ca63127d2017e598680 Mon Sep 17 00:00:00 2001 From: Marijn Kruisselbrink Date: Thu, 29 Apr 2021 19:00:26 +0000 Subject: [Backport] CVE-2021-30515: Use after free in File API. Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2883604: FileAPI: Terminate FileReaderLoader before dispatching onabort event. Otherwise FileReader could end up in an inconsistent state where a load is still in progress while the state was set to done. Bug: 1201073 Change-Id: Ib2c833537e1badc57d125568d5d35f53f12582a8 Reviewed-by: Austin Sullivan Commit-Queue: Marijn Kruisselbrink Cr-Commit-Position: refs/heads/master@{#877579} Reviewed-by: Allan Sandfeld Jensen --- .../third_party/blink/renderer/core/fileapi/file_reader.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/chromium/third_party/blink/renderer/core/fileapi/file_reader.cc b/chromium/third_party/blink/renderer/core/fileapi/file_reader.cc index d2e87ac7ddf..960d796c218 100644 --- a/chromium/third_party/blink/renderer/core/fileapi/file_reader.cc +++ b/chromium/third_party/blink/renderer/core/fileapi/file_reader.cc @@ -332,7 +332,10 @@ void FileReader::abort() { loading_state_ = kLoadingStateAborted; DCHECK_NE(kDone, state_); - state_ = kDone; + // Synchronously cancel the loader before dispatching events. This way we make + // sure the FileReader internal state stays consistent even if another load + // is started from one of the event handlers, or right after abort returns. + Terminate(); base::AutoReset firing_events(&still_firing_events_, true); @@ -344,15 +347,12 @@ void FileReader::abort() { ThrottlingController::RemoveReader(GetExecutionContext(), this); FireEvent(EventTypeNames::abort); + // TODO(https://crbug.com/1204139): Only fire loadend event if no new load was + // started from the abort event handler. FireEvent(EventTypeNames::loadend); // All possible events have fired and we're done, no more pending activity. ThrottlingController::FinishReader(GetExecutionContext(), this, final_step); - - // Also synchronously cancel the loader, as script might initiate a new load - // right after this method returns, in which case an async termination would - // terminate the wrong loader. - Terminate(); } void FileReader::result(ScriptState* state, @@ -425,6 +425,8 @@ void FileReader::DidFinishLoading() { ThrottlingController::RemoveReader(GetExecutionContext(), this); FireEvent(EventTypeNames::load); + // TODO(https://crbug.com/1204139): Only fire loadend event if no new load was + // started from the abort event handler. FireEvent(EventTypeNames::loadend); // All possible events have fired and we're done, no more pending activity. -- cgit v1.2.1