github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: OOB access in ReadableStream::Close (#22435)

Browse Source 
* fix: move ReadableStream requests onto the stack before iteration

* fix: Streams: Convert state DCHECKs to CHECKs

* fix patches
This commit is contained in:
Jeremy Apthorp
2020-02-27 15:53:01 -08:00
committed by GitHub
parent 1bde50636f
commit f29a600fd8
3 changed files with 316 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
patches/chromium/.patches
View File

@@ -95,3 +95,5 @@ never_let_a_non-zero-size_pixel_snap_to_zero_size.patch
fix_hi-dpi_transitions_on_catalina.patch
typemap.patch
allow_restricted_clock_nanosleep_in_linux_sandbox.patch
move_readablestream_requests_onto_the_stack_before_iteration.patch
streams_convert_state_dchecks_to_checks.patch

55
patches/chromium/move_readablestream_requests_onto_the_stack_before_iteration.patch Normal file
View File

@@ -0,0 +1,55 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp <nornagon@nornagon.net>
Date: Thu, 27 Feb 2020 12:40:02 -0800
Subject: Move ReadableStream requests onto the stack before iteration.
This might be subject to concurrent modification by script.
Cherry-picked from 12310ed05f15fea5fa6824c6a6b5d86f81532e25.
diff --git a/third_party/blink/renderer/core/streams/readable_stream_native.cc b/third_party/blink/renderer/core/streams/readable_stream_native.cc
index 632ea367d6861cc45625d3ff305f21249ccd6db6..9f2f7b8cbef30e8c69cb3d4ca1c5b74d6b0073e4 100644
--- a/third_party/blink/renderer/core/streams/readable_stream_native.cc
+++ b/third_party/blink/renderer/core/streams/readable_stream_native.cc
@@ -708,7 +708,9 @@ class ReadableStreamNative::TeeEngine::PullAlgorithm final
// b. Perform ! ReadableStreamDefaultControllerClose(branch2.
// [[readableStreamController]]).
for (int branch = 0; branch < 2; ++branch) {
- if (!engine_->canceled_[branch]) {
+ if (!engine_->canceled_[branch] &&
+ ReadableStreamDefaultController::CanCloseOrEnqueue(
+ engine_->controller_[branch])) {
ReadableStreamDefaultController::Close(
script_state, engine_->controller_[branch]);
}
@@ -734,7 +736,9 @@ class ReadableStreamNative::TeeEngine::PullAlgorithm final
// ReadableStreamDefaultControllerEnqueue(branch2.
// [[readableStreamController]], value2).
for (int branch = 0; branch < 2; ++branch) {
- if (!engine_->canceled_[branch]) {
+ if (!engine_->canceled_[branch] &&
+ ReadableStreamDefaultController::CanCloseOrEnqueue(
+ engine_->controller_[branch])) {
ReadableStreamDefaultController::Enqueue(script_state,
engine_->controller_[branch],
value, exception_state);
@@ -1532,7 +1536,9 @@ void ReadableStreamNative::Close(ScriptState* script_state,
// 5. If ! IsReadableStreamDefaultReader(reader) is true,
// a. Repeat for each readRequest that is an element of reader.
// [[readRequests]],
- for (StreamPromiseResolver* promise : reader->read_requests_) {
+ HeapDeque<Member<StreamPromiseResolver>> requests;
+ requests.Swap(reader->read_requests_);
+ for (StreamPromiseResolver* promise : requests) {
// i. Resolve readRequest.[[promise]] with !
// ReadableStreamCreateReadResult(undefined, true, reader.
// [[forAuthorCode]]).
@@ -1543,7 +1549,7 @@ void ReadableStreamNative::Close(ScriptState* script_state,
}
// b. Set reader.[[readRequests]] to an empty List.
- reader->read_requests_.clear();
+ // This is not required since we've already called Swap().
// 6. Resolve reader.[[closedPromise]] with undefined.
reader->closed_promise_->ResolveWithUndefined(script_state);

259
patches/chromium/streams_convert_state_dchecks_to_checks.patch Normal file
View File

@@ -0,0 +1,259 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp <nornagon@nornagon.net>
Date: Thu, 27 Feb 2020 12:53:52 -0800
Subject: Streams: Convert state DCHECKs to CHECKs
For "cheap" checks of state in the streams implementation, use CHECKs
instead of DCHECKs. This will improve robustness against logic errors.
Cherry-picked from 122b074f0354079f3d9044cc14890dcfd2d72918.
diff --git a/third_party/blink/renderer/core/streams/readable_stream_default_controller.cc b/third_party/blink/renderer/core/streams/readable_stream_default_controller.cc
index 653cab070a3e995ea62d1ca5d604d6b674089efe..d9e69a72280f4f098ebe25d99f90806a07ca71f4 100644
--- a/third_party/blink/renderer/core/streams/readable_stream_default_controller.cc
+++ b/third_party/blink/renderer/core/streams/readable_stream_default_controller.cc
@@ -110,7 +110,7 @@ void ReadableStreamDefaultController::Close(
// 2. Assert: ! ReadableStreamDefaultControllerCanCloseOrEnqueue(controller)
// is true.
- DCHECK(CanCloseOrEnqueue(controller));
+ CHECK(CanCloseOrEnqueue(controller));
// 3. Set controller.[[closeRequested]] to true.
controller->is_close_requested_ = true;
@@ -136,7 +136,7 @@ void ReadableStreamDefaultController::Enqueue(
// 2. Assert: ! ReadableStreamDefaultControllerCanCloseOrEnqueue(controller)
// is true.
- DCHECK(CanCloseOrEnqueue(controller));
+ CHECK(CanCloseOrEnqueue(controller));
// 3. If ! IsReadableStreamLocked(stream) is true and !
// ReadableStreamGetNumReadRequests(stream) > 0, perform !
@@ -261,7 +261,7 @@ const char* ReadableStreamDefaultController::EnqueueExceptionMessage(
if (state == ReadableStreamNative::kErrored) {
return "Cannot enqueue a chunk into an errored readable stream";
}
- DCHECK(state == ReadableStreamNative::kClosed);
+ CHECK(state == ReadableStreamNative::kClosed);
return "Cannot enqueue a chunk into a closed readable stream";
}
diff --git a/third_party/blink/renderer/core/streams/readable_stream_native.cc b/third_party/blink/renderer/core/streams/readable_stream_native.cc
index 9f2f7b8cbef30e8c69cb3d4ca1c5b74d6b0073e4..0c5bbb02ccbb2ac7b60acb13117ea43880c621cf 100644
--- a/third_party/blink/renderer/core/streams/readable_stream_native.cc
+++ b/third_party/blink/renderer/core/streams/readable_stream_native.cc
@@ -1333,7 +1333,7 @@ void ReadableStreamNative::Initialize(ReadableStreamNative* stream) {
// initialised correctly.
// https://streams.spec.whatwg.org/#initialize-readable-stream
// 1. Set stream.[[state]] to "readable".
- DCHECK_EQ(stream->state_, kReadable);
+ CHECK_EQ(stream->state_, kReadable);
// 2. Set stream.[[reader]] and stream.[[storedError]] to undefined.
DCHECK(!stream->reader_);
DCHECK(stream->stored_error_.IsEmpty());
@@ -1452,7 +1452,7 @@ StreamPromiseResolver* ReadableStreamNative::AddReadRequest(
DCHECK(stream->reader_);
// 2. Assert: stream.[[state]] is "readable".
- DCHECK_EQ(stream->state_, kReadable);
+ CHECK_EQ(stream->state_, kReadable);
// 3. Let promise be a new promise.
auto* promise = MakeGarbageCollected<StreamPromiseResolver>(script_state);
@@ -1519,7 +1519,7 @@ void ReadableStreamNative::Close(ScriptState* script_state,
ReadableStreamNative* stream) {
// https://streams.spec.whatwg.org/#readable-stream-close
// 1. Assert: stream.[[state]] is "readable".
- DCHECK_EQ(stream->state_, kReadable);
+ CHECK_EQ(stream->state_, kReadable);
// 2. Set stream.[[state]] to "closed".
stream->state_ = kClosed;
@@ -1606,7 +1606,7 @@ void ReadableStreamNative::Error(ScriptState* script_state,
v8::Local<v8::Value> e) {
// https://streams.spec.whatwg.org/#readable-stream-error
// 2. Assert: stream.[[state]] is "readable".
- DCHECK_EQ(stream->state_, kReadable);
+ CHECK_EQ(stream->state_, kReadable);
auto* isolate = script_state->GetIsolate();
// 3. Set stream.[[state]] to "errored".
diff --git a/third_party/blink/renderer/core/streams/transform_stream_native.cc b/third_party/blink/renderer/core/streams/transform_stream_native.cc
index 073efffc0ccad31384a29cd7a0d4863dc22540b2..955e1f21559e8c7a51da0692f05f90e1203023ad 100644
--- a/third_party/blink/renderer/core/streams/transform_stream_native.cc
+++ b/third_party/blink/renderer/core/streams/transform_stream_native.cc
@@ -489,7 +489,7 @@ class TransformStreamNative::DefaultSinkWriteAlgorithm final
}
// 4. Assert: state is "writable".
- DCHECK(writable->IsWritable());
+ CHECK(writable->IsWritable());
// 5. Return ! TransformStreamDefaultControllerPerformTransform(
// controller, chunk).
diff --git a/third_party/blink/renderer/core/streams/writable_stream_default_controller.cc b/third_party/blink/renderer/core/streams/writable_stream_default_controller.cc
index 08652a65c0a142031890a7a0005babf08162f41a..afded82d46da46a156a87be2379c428824b53e66 100644
--- a/third_party/blink/renderer/core/streams/writable_stream_default_controller.cc
+++ b/third_party/blink/renderer/core/streams/writable_stream_default_controller.cc
@@ -146,8 +146,8 @@ void WritableStreamDefaultController::SetUp(
// 16. Upon fulfillment of startPromise
// a. Assert: stream.[[state]] is "writable" or "erroring".
const auto state = stream_->GetState();
- DCHECK(state == WritableStreamNative::kWritable ||
- state == WritableStreamNative::kErroring);
+ CHECK(state == WritableStreamNative::kWritable ||
+ state == WritableStreamNative::kErroring);
// b. Set controller.[[started]] to true.
WritableStreamDefaultController* controller = stream_->Controller();
@@ -178,8 +178,8 @@ void WritableStreamDefaultController::SetUp(
// 17. Upon rejection of startPromise with reason r,
// a. Assert: stream.[[state]] is "writable" or "erroring".
const auto state = stream_->GetState();
- DCHECK(state == WritableStreamNative::kWritable ||
- state == WritableStreamNative::kErroring);
+ CHECK(state == WritableStreamNative::kWritable ||
+ state == WritableStreamNative::kErroring);
// b. Set controller.[[started]] to true.
WritableStreamDefaultController* controller = stream_->Controller();
@@ -578,8 +578,8 @@ void WritableStreamDefaultController::ProcessWrite(
const auto state = stream_->GetState();
// c. Assert: state is "writable" or "erroring".
- DCHECK(state == WritableStreamNative::kWritable ||
- state == WritableStreamNative::kErroring);
+ CHECK(state == WritableStreamNative::kWritable ||
+ state == WritableStreamNative::kErroring);
// d. Perform ! DequeueValue(controller).
controller_->queue_->DequeueValue(script_state->GetIsolate());
diff --git a/third_party/blink/renderer/core/streams/writable_stream_default_writer.cc b/third_party/blink/renderer/core/streams/writable_stream_default_writer.cc
index 0d78ec2429f6be6c38e4f88e8b4d146257791918..13511cda376d642b0fc7036de0f715903106350d 100644
--- a/third_party/blink/renderer/core/streams/writable_stream_default_writer.cc
+++ b/third_party/blink/renderer/core/streams/writable_stream_default_writer.cc
@@ -347,8 +347,8 @@ v8::Local<v8::Promise> WritableStreamDefaultWriter::CloseWithErrorPropagation(
}
// 6. Assert: state is "writable" or "erroring".
- DCHECK(state == WritableStreamNative::kWritable ||
- state == WritableStreamNative::kErroring);
+ CHECK(state == WritableStreamNative::kWritable ||
+ state == WritableStreamNative::kErroring);
// 7. Return ! WritableStreamDefaultWriterClose(writer).
return Close(script_state, writer);
@@ -539,11 +539,11 @@ v8::Local<v8::Promise> WritableStreamDefaultWriter::Close(
}
// 5. Assert: state is "writable" or "erroring".
- DCHECK(state == WritableStreamNative::kWritable ||
- state == WritableStreamNative::kErroring);
+ CHECK(state == WritableStreamNative::kWritable ||
+ state == WritableStreamNative::kErroring);
// 6. Assert: ! WritableStreamCloseQueuedOrInFlight(stream) is false.
- DCHECK(!WritableStreamNative::CloseQueuedOrInFlight(stream));
+ CHECK(!WritableStreamNative::CloseQueuedOrInFlight(stream));
// 7. Let promise be a new promise.
auto* promise = MakeGarbageCollected<StreamPromiseResolver>(script_state);
diff --git a/third_party/blink/renderer/core/streams/writable_stream_native.cc b/third_party/blink/renderer/core/streams/writable_stream_native.cc
index 913ddadae5ac981115397db59ced04e847b9a037..9f272dcd791a32590e931141db9ebb6eda103113 100644
--- a/third_party/blink/renderer/core/streams/writable_stream_native.cc
+++ b/third_party/blink/renderer/core/streams/writable_stream_native.cc
@@ -319,7 +319,7 @@ v8::Local<v8::Promise> WritableStreamNative::Abort(
}
// 4. Assert: state is "writable" or "erroring".
- DCHECK(state == kWritable || state == kErroring);
+ CHECK(state == kWritable || state == kErroring);
// 5. Let wasAlreadyErroring be false.
// 6. If state is "erroring",
@@ -358,7 +358,7 @@ v8::Local<v8::Promise> WritableStreamNative::AddWriteRequest(
DCHECK(IsLocked(stream));
// 2. Assert: stream.[[state]] is "writable".
- DCHECK_EQ(stream->state_, kWritable);
+ CHECK_EQ(stream->state_, kWritable);
// 3. Let promise be a new promise.
auto* promise = MakeGarbageCollected<StreamPromiseResolver>(script_state);
@@ -396,7 +396,7 @@ void WritableStreamNative::DealWithRejection(ScriptState* script_state,
}
// 3. Assert: state is "erroring".
- DCHECK_EQ(state, kErroring);
+ CHECK_EQ(state, kErroring);
// 4. Perform ! WritableStreamFinishErroring(stream).
FinishErroring(script_state, stream);
@@ -410,7 +410,7 @@ void WritableStreamNative::StartErroring(ScriptState* script_state,
DCHECK(stream->stored_error_.IsEmpty());
// 2. Assert: stream.[[state]] is "writable".
- DCHECK_EQ(stream->state_, kWritable);
+ CHECK_EQ(stream->state_, kWritable);
// 3. Let controller be stream.[[writableStreamController]].
WritableStreamDefaultController* controller =
@@ -447,7 +447,7 @@ void WritableStreamNative::FinishErroring(ScriptState* script_state,
WritableStreamNative* stream) {
// https://streams.spec.whatwg.org/#writable-stream-finish-erroring
// 1. Assert: stream.[[state]] is "erroring".
- DCHECK_EQ(stream->state_, kErroring);
+ CHECK_EQ(stream->state_, kErroring);
// 2. Assert: ! WritableStreamHasOperationMarkedInFlight(stream) is false.
DCHECK(!HasOperationMarkedInFlight(stream));
@@ -596,7 +596,7 @@ void WritableStreamNative::FinishInFlightWriteWithError(
// 4. Assert: stream.[[state]] is "writable" or "erroring".
const auto state = stream->state_;
- DCHECK(state == kWritable || state == kErroring);
+ CHECK(state == kWritable || state == kErroring);
// 5. Perform ! WritableStreamDealWithRejection(stream, error).
DealWithRejection(script_state, stream, error);
@@ -618,7 +618,7 @@ void WritableStreamNative::FinishInFlightClose(ScriptState* script_state,
const auto state = stream->state_;
// 5. Assert: stream.[[state]] is "writable" or "erroring".
- DCHECK(state == kWritable || state == kErroring);
+ CHECK(state == kWritable || state == kErroring);
// 6. If state is "erroring",
if (state == kErroring) {
@@ -672,7 +672,7 @@ void WritableStreamNative::FinishInFlightCloseWithError(
// 4. Assert: stream.[[state]] is "writable" or "erroring".
const auto state = stream->state_;
- DCHECK(state == kWritable || state == kErroring);
+ CHECK(state == kWritable || state == kErroring);
// 5. If stream.[[pendingAbortRequest]] is not undefined,
if (stream->pending_abort_request_) {
@@ -728,10 +728,10 @@ void WritableStreamNative::UpdateBackpressure(ScriptState* script_state,
bool backpressure) {
// https://streams.spec.whatwg.org/#writable-stream-update-backpressure
// 1. Assert: stream.[[state]] is "writable".
- DCHECK_EQ(stream->state_, kWritable);
+ CHECK_EQ(stream->state_, kWritable);
// 2. Assert: ! WritableStreamCloseQueuedOrInFlight(stream) is false.
- DCHECK(!CloseQueuedOrInFlight(stream));
+ CHECK(!CloseQueuedOrInFlight(stream));
// 3. Let writer be stream.[[writer]].
WritableStreamDefaultWriter* writer = stream->writer_;
@@ -803,7 +803,7 @@ void WritableStreamNative::RejectCloseAndClosedPromiseIfNeeded(
WritableStreamNative* stream) {
// https://streams.spec.whatwg.org/#writable-stream-reject-close-and-closed-promise-if-needed
// // 1. Assert: stream.[[state]] is "errored".
- DCHECK_EQ(stream->state_, kErrored);
+ CHECK_EQ(stream->state_, kErrored);
auto* isolate = script_state->GetIsolate();