[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 <asully@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#877579}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files 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<bool> 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.