Segmentation fault when using inspector on experimental permission enabled process

[thezzisu]

Version

v22.2.0

Platform

Linux AS-ZISU 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

> cat index.mjs
import {setTimeout} from 'node:timers/promises'
await setTimeout(1000000)

> node --experimental-permission --allow-fs-read=\* --inspect-brk index.mjs
(node:398141) ExperimentalWarning: Permission is an experimental feature
(Use `node --trace-warnings ...` to show where the warning was created)
[1]    398141 segmentation fault  node --experimental-permission --allow-fs-read=\* --inspect-brk index.mjs

The content of the script does not matter - even an empty script will work.

How often does it reproduce? Is there a required condition?

No condition required. Node v22.2.0 will crash everytime when --experimental-permission is combined with --inspect-brk.

What is the expected behavior? Why is that the expected behavior?

The inspector should work and do not segfault.

What do you see instead?

The node process crash because of segfault. Also, no stack trace is printed.

Additional information

The experimental process-based permission model have unexpected side effects with inspector, which is not well-documented.

When enabled, --inspect and SIGUSR1 both wouldn't work. Should this be a feature, or another bug?

[targos]

I don't know how to debug C++ code, but here's some info:

Process 84595 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x58)
    frame #0: 0x00000001004f4d78 node`std::__1::__hash_table<std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::__unordered_map_hasher<int, std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::hash<int>, std::__1::equal_to<int>, true>, std::__1::__unordered_map_equal<int, std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::equal_to<int>, std::__1::hash<int>, true>, std::__1::allocator<std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>>>::begin(this=0x0000000000000048) at __hash_table:1561:35
   1558	typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator
   1559	__hash_table<_Tp, _Hash, _Equal, _Alloc>::begin() _NOEXCEPT
   1560	{
-> 1561	    return iterator(__p1_.first().__next_);
   1562	}
   1563
   1564	template <class _Tp, class _Hash, class _Equal, class _Alloc>
Target 0: (node) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x58)
  * frame #0: 0x00000001004f4d78 node`std::__1::__hash_table<std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::__unordered_map_hasher<int, std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::hash<int>, std::__1::equal_to<int>, true>, std::__1::__unordered_map_equal<int, std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>, std::__1::equal_to<int>, std::__1::hash<int>, true>, std::__1::allocator<std::__1::__hash_value_type<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>>>::begin(this=0x0000000000000048) at __hash_table:1561:35
    frame #1: 0x00000001004f4b94 node`std::__1::unordered_map<int, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>, std::__1::hash<int>, std::__1::equal_to<int>, std::__1::allocator<std::__1::pair<int const, std::__1::unique_ptr<node::inspector::(anonymous namespace)::ChannelImpl, std::__1::default_delete<node::inspector::(anonymous namespace)::ChannelImpl>>>>>::begin[abi:un170006](this=0x0000000000000048 size=0) at unordered_map:1297:62
    frame #2: 0x00000001004f1e5c node`node::inspector::NodeInspectorClient::schedulePauseOnNextStatement(this=0x0000000000000000, reason="Break on start") at inspector_agent.cc:609:33
    frame #3: 0x00000001004f1e28 node`node::inspector::Agent::PauseOnNextJavascriptStatement(this=0x0000600003ab8b00, reason="Break on start") at inspector_agent.cc:892:12
    frame #4: 0x000000010051a3f8 node`node::inspector::(anonymous namespace)::CallAndPauseOnStart(args=0x000000016fdf5938) at inspector_js_api.cc:187:27

[targos]

@nodejs/inspector

[eugeneo]

I confirmed the problem, will look into it, assigning to myself.

[eugeneo]

Oh, can't assign, no longer a contributor :) Feel free to assign to me.

[theanarkh]

I have made a PR to fix it.

[theanarkh]

I think this is because setting --experimental-permission flag will disable inspector(experimental_permission,
should_create_inspector), then calling callAndPauseOnStart during startup make the process crash.

[benjamingr]

Oh, can't assign, no longer a contributor :) Feel free to assign to me.

Kind reminder that collaborators removed due to inactivity are more than welcome to come back and many have done so in the past :)

[cola119]

When enabled, --inspect and SIGUSR1 both wouldn't work. Should this be a feature, or another bug?

@RafaelGSS According to RafaelGSS@34d92ed, the inspector feature is restricted when the permission model is enabled, and there is no flag to enable it, correct?

[RafaelGSS]

That's correct. We haven't included a --allow-inspector flag as it's pretty much the same as disabling the permission model. There are some ways to bypass --allow-fs restrictions through inspector protocol, for instance. But, I'm happy to discuss adding this cli option on nodejs. Feel free to open an issue on security-wg repository.

[eugeneo]

There used to be discussions with some cloud providers about optionally restricting some Inspector domains to make some operations impossible. E.g. we could disable Debugger/Runtime but leave Tracing/Profile. It should be fairly doable from a technical point of view, though I expect there would be a game of wack-a-mole with some functionality still providing too much power...

Main concern was the worry that most frontends would not expect the domains being unavailable, making this functionality mostly useless.

Feel free to initiate a discussion if there's more interest in this.