Skip to content

Commit

Permalink
src: handle fatal error when Environment is not assigned to context
Browse files Browse the repository at this point in the history
Previously when an uncaught JS error is thrown before Environment was
assigned to the context (e.g. a SyntaxError in a per-context script),
it triggered an infinite recursion:

1. The error message listener `node::OnMessage()` triggered
   `node::FatalException()`
2. `node::FatalException()` attempted to get the Environment
   assigned to the context entered using `Environment::GetCurrent()`
3. `Environment::GetCurrent()` previously incorrectly accepted
   out-of-bound access with the length of the embedder data array
   as index, and called `context->GetAlignedPointerFromEmbedderData()`
4. The out-of-bound access in `GetAlignedPointerFromEmbedderData()`
   triggered a fatal error, which was handled by `node::FatalError()`
5. `node::FatalError()` called `Environment::GetCurrent()`, then
   we went back to 3.

This patch fixes the incorrect guard in 3. When
`Environment::GetCurrent()` returns nullptr (when Environment is not
yet assigned to the context) in 2, it now prints the JS stack trace
and crashes directly.

PR-URL: #27236
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
joyeecheung committed Apr 17, 2019
1 parent 83d1ca7 commit cdba9f2
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
17 changes: 10 additions & 7 deletions src/env-inl.h
Expand Up @@ -302,15 +302,18 @@ inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {
}

inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
if (UNLIKELY(context.IsEmpty() ||
context->GetNumberOfEmbedderDataFields() <
ContextEmbedderIndex::kContextTag ||
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kContextTag) !=
Environment::kNodeContextTagPtr)) {
if (UNLIKELY(context.IsEmpty())) {
return nullptr;
}
if (UNLIKELY(context->GetNumberOfEmbedderDataFields() <=
ContextEmbedderIndex::kContextTag)) {
return nullptr;
}
if (UNLIKELY(context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kContextTag) !=
Environment::kNodeContextTagPtr)) {
return nullptr;
}

return static_cast<Environment*>(
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kEnvironment));
Expand Down
39 changes: 29 additions & 10 deletions src/node_errors.cc
Expand Up @@ -6,6 +6,7 @@
#ifdef NODE_REPORT
#include "node_report.h"
#endif
#include "node_v8_platform-inl.h"

namespace node {

Expand Down Expand Up @@ -171,21 +172,27 @@ void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
fflush(stderr);
}

void PrintCaughtException(Isolate* isolate,
Local<Context> context,
const v8::TryCatch& try_catch) {
CHECK(try_catch.HasCaught());
Local<Value> err = try_catch.Exception();
Local<Message> message = try_catch.Message();
Local<v8::StackTrace> stack = message->GetStackTrace();
void PrintException(Isolate* isolate,
Local<Context> context,
Local<Value> err,
Local<Message> message) {
node::Utf8Value reason(isolate,
err->ToDetailString(context).ToLocalChecked());
bool added_exception_line = false;
std::string source =
GetErrorSource(isolate, context, message, &added_exception_line);
fprintf(stderr, "%s\n", source.c_str());
fprintf(stderr, "%s\n", *reason);
PrintStackTrace(isolate, stack);

Local<v8::StackTrace> stack = message->GetStackTrace();
if (!stack.IsEmpty()) PrintStackTrace(isolate, stack);
}

void PrintCaughtException(Isolate* isolate,
Local<Context> context,
const v8::TryCatch& try_catch) {
CHECK(try_catch.HasCaught());
PrintException(isolate, context, try_catch.Exception(), try_catch.Message());
}

void AppendExceptionLine(Environment* env,
Expand Down Expand Up @@ -777,8 +784,20 @@ void FatalException(Isolate* isolate,
CHECK(!error.IsEmpty());
HandleScope scope(isolate);

Environment* env = Environment::GetCurrent(isolate);
CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here.
CHECK(isolate->InContext());
Local<Context> context = isolate->GetCurrentContext();
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) {
// This means that the exception happens before Environment is assigned
// to the context e.g. when there is a SyntaxError in a per-context
// script - which usually indicates that there is a bug because no JS
// error is supposed to be thrown at this point.
// Since we don't have access to Environment here, there is not
// much we can do, so we just print whatever is useful and crash.
PrintException(isolate, context, error, message);
Abort();
}

Local<Object> process_object = env->process_object();
Local<String> fatal_exception_string = env->fatal_exception_string();
Local<Value> fatal_exception_function =
Expand Down

0 comments on commit cdba9f2

Please sign in to comment.