NeedImmediateCallbackSetter terminates after setting global.process

Version: 8.9.3 (LTS)
Platform: Windows 10 Professional 64 bit
Subsystem: process

This causes node 8.9.3 to crash:

> global.process = { __proto__: global.process, pid: 123456 }
process { pid: 123456 }
> process._needImmediateCallback = true
true
> FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.

I came across this problem when executing unit tests of an npm package which intentionally replaces the process object. The problem seems to be caused by CheckImmediate which is called by NeedImmediateCallbackSetter via libuv:

node/src/node.cc

Lines 379 to 384 in ab46b8e

    
           MakeCallback(env->isolate(), 
        
                        env->process_object(), 
        
                        env->immediate_callback_string(), 
        
                        0, 
        
                        nullptr, 
        
                        {0, 0}).ToLocalChecked();

MakeCallback fails because callback_v is not a function:

node/src/node.cc

Lines 1528 to 1539 in ab46b8e

    
           MaybeLocal<Value> MakeCallback(Isolate* isolate, 
        
                                          Local<Object> recv, 
        
                                          Local<String> symbol, 
        
                                          int argc, 
        
                                          Local<Value> argv[], 
        
                                          async_context asyncContext) { 
        
             Local<Value> callback_v = recv->Get(symbol); 
        
             if (callback_v.IsEmpty()) return Local<Value>(); 
        
             if (!callback_v->IsFunction()) return Local<Value>(); 
        
             Local<Function> callback = callback_v.As<Function>(); 
        
             return MakeCallback(isolate, recv, callback, argc, argv, asyncContext); 
        
           }

node 9 is not affected.

MakeCallback can return an empty Local, so CheckImmediate should check this instead of calling ToLocalChecked

@addaleax

cc @addaleax

A better solution is to remove process._immediateCallback and introduce some internal API that can't be tampered with. Shouldn't be too hard, there's only place in lib/timers.js that uses it.

timers: make setImmediate() immune to tampering

#17736

timers: make setImmediate() immune to tampering

Navigation Menu

Repository navigation

NeedImmediateCallbackSetter terminates after setting global.process #17681

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	MakeCallback(env->isolate(),
	env->process_object(),
	env->immediate_callback_string(),
	0,
	nullptr,
	{0, 0}).ToLocalChecked();

	MaybeLocal<Value> MakeCallback(Isolate* isolate,
	Local<Object> recv,
	Local<String> symbol,
	int argc,
	Local<Value> argv[],
	async_context asyncContext) {
	Local<Value> callback_v = recv->Get(symbol);
	if (callback_v.IsEmpty()) return Local<Value>();
	if (!callback_v->IsFunction()) return Local<Value>();
	Local<Function> callback = callback_v.As<Function>();
	return MakeCallback(isolate, recv, callback, argc, argv, asyncContext);
	}

NeedImmediateCallbackSetter terminates after setting global.process #17681

Description

Activity

targos commented on Dec 14, 2017

tniessen commented on Dec 14, 2017

bnoordhuis commented on Dec 15, 2017

bnoordhuis commented on Dec 18, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions