AddressSanitizer: container-overflow during `package.json` resolution with `simdjson`

[codebytere]

Version

main

Platform

Linux 5469246cac8b 6.5.0-1025-azure #26~22.04.1-Ubuntu SMP Thu Jul 11 22:33:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

1. Clone https://gist.github.com/codebytere/3dedc548edcc0cc218d1ec85d8b61b02
2. Enter the directory and install modules with npm i
3. Use an asan-enabled Node.js build to run the file
4. Observe ASAN crash

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

Every time.

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

No crash.

What do you see instead?

=================================================================
==625312==ERROR: AddressSanitizer: container-overflow on address 0x7bf653f0e7b7 at pc 0x5891de703a35 bp 0x7fffc7b5e470 sp 0x7fffc7b5e468
READ of size 32 at 0x7bf653f0e7b7 thread T0
    #0 0x5891de703a34 in load third_party/electron_node/deps/simdjson/simdjson.cpp:14372:14
    #1 0x5891de703a34 in simd8 third_party/electron_node/deps/simdjson/simdjson.cpp:14525:61
    #2 0x5891de703a34 in copy_and_find third_party/electron_node/deps/simdjson/simdjson.cpp:14699:18
    #3 0x5891de703a34 in parse_string third_party/electron_node/deps/simdjson/simdjson.cpp:19596:21
    #4 0x5891de703a34 in simdjson::haswell::dom_parser_implementation::parse_string(unsigned char const*, unsigned char*, bool) const third_party/electron_node/deps/simdjson/simdjson.cpp:20193:10
    #5 0x5891de1bb79f in unescape third_party/electron_node/deps/simdjson/simdjson.h:49904:34
    #6 0x5891de1bb79f in unescape third_party/electron_node/deps/simdjson/simdjson.h:48874:18
    #7 0x5891de1bb79f in unescape third_party/electron_node/deps/simdjson/simdjson.h:50092:15
    #8 0x5891de1bb79f in unescape third_party/electron_node/deps/simdjson/simdjson.h:50130:16
    #9 0x5891de1bb79f in get_string third_party/electron_node/deps/simdjson/simdjson.h:51535:32
    #10 0x5891de1bb79f in get_string<std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > > > third_party/electron_node/deps/simdjson/simdjson.h:51540:14
    #11 0x5891de1bb79f in get_string<std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > > > third_party/electron_node/deps/simdjson/simdjson.h:50528:15
    #12 0x5891de1bb79f in node::modules::BindingData::GetPackageJSON(node::Realm*, std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, node::modules::BindingData::ErrorContext*) third_party/electron_node/src/node_modules.cc:162:17
    #13 0x5891de1c0b8f in node::modules::BindingData::ReadPackageJSON(v8::FunctionCallbackInfo<v8::Value> const&) third_party/electron_node/src/node_modules.cc:271:23
    #14 0x5891d61e586d in Builtins_CallApiCallbackGeneric setup-isolate-deserialize.cc
    #15 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #16 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #17 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #18 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #19 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #20 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #21 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #22 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #23 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #24 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #25 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #26 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #27 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #28 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #29 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #30 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #31 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #32 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #33 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #34 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #35 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #36 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #37 0x5891d61e0fdb in Builtins_JSEntryTrampoline setup-isolate-deserialize.cc
    #38 0x5891d61e0d1e in Builtins_JSEntry setup-isolate-deserialize.cc
    #39 0x5891d11e9fc3 in Call v8/src/execution/simulator.h:191:12
    #40 0x5891d11e9fc3 in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) v8/src/execution/execution.cc:420:22
    #41 0x5891d11e8c4a in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) v8/src/execution/execution.cc:506:10
    #42 0x5891d0976e50 in v8::Function::Call(v8::Isolate*, v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) v8/src/api/api.cc:5601:7
    #43 0x5891de04d430 in CompileAndCall third_party/electron_node/src/node_builtins.cc:497:14
    #44 0x5891de04d430 in node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, node::Realm*) third_party/electron_node/src/node_builtins.cc:481:10
    #45 0x5891de25c7f8 in node::Realm::ExecuteBootstrapper(char const*) third_party/electron_node/src/node_realm.cc:161:32
    #46 0x5891ddfd9ba0 in node::StartExecution(node::Environment*, char const*) third_party/electron_node/src/node.cc:297:35
    #47 0x5891ddfd96bc in node::StartExecution(node::Environment*, std::__Cr::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) third_party/electron_node/src/node.cc
    #48 0x5891dde03a72 in node::LoadEnvironment(node::Environment*, std::__Cr::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>, std::__Cr::function<void (node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Value>)>) third_party/electron_node/src/api/environment.cc:534:10
    #49 0x5891c9464568 in electron::NodeMain(int, char**) electron/shell/app/node_main.cc:287:5
    #50 0x5891c945759a in main electron/shell/app/electron_main_linux.cc:34:12
    #51 0x7e96559e2082 in __libc_start_main /build/glibc-LcI20x/glibc-2.31/csu/../csu/libc-start.c:308:16

0x7bf653f0e7b7 is located 567 bytes inside of 616-byte region [0x7bf653f0e580,0x7bf653f0e7e8)
allocated by thread T0 here:
    #0 0x5891c93fdbdd in operator new(unsigned long) /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
    #1 0x5891c9d2876f in __libcpp_operator_new<unsigned long> third_party/libc++/src/include/new:277:10
    #2 0x5891c9d2876f in __libcpp_allocate third_party/libc++/src/include/new:301:10
    #3 0x5891c9d2876f in allocate third_party/libc++/src/include/__memory/allocator.h:103:32
    #4 0x5891c9d2876f in __allocate_at_least<std::__Cr::allocator<char> > third_party/libc++/src/include/__memory/allocate_at_least.h:41:19
    #5 0x5891c9d2876f in std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>::__shrink_or_extend(unsigned long) third_party/libc++/src/include/string
    #6 0x5891de1ba698 in iterate third_party/electron_node/deps/simdjson/simdjson.h:49828:10
    #7 0x5891de1ba698 in node::modules::BindingData::GetPackageJSON(node::Realm*, std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, node::modules::BindingData::ErrorContext*) third_party/electron_node/src/node_modules.cc:107:33
    #8 0x5891de1c0b8f in node::modules::BindingData::ReadPackageJSON(v8::FunctionCallbackInfo<v8::Value> const&) third_party/electron_node/src/node_modules.cc:271:23
    #9 0x5891d61e586d in Builtins_CallApiCallbackGeneric setup-isolate-deserialize.cc
    #10 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #11 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #12 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #13 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #14 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #15 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #16 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #17 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #18 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #19 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #20 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #21 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #22 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #23 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #24 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #25 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #26 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #27 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #28 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #29 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #30 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #31 0x5891d61e3a1d in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #32 0x5891d61e0fdb in Builtins_JSEntryTrampoline setup-isolate-deserialize.cc
    #33 0x5891d61e0d1e in Builtins_JSEntry setup-isolate-deserialize.cc
    #34 0x5891d11e9fc3 in Call v8/src/execution/simulator.h:191:12
    #35 0x5891d11e9fc3 in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) v8/src/execution/execution.cc:420:22

HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_container_overflow=0.
If you suspect a false positive see also: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow.
SUMMARY: AddressSanitizer: container-overflow third_party/electron_node/deps/simdjson/simdjson.cpp:14372:14 in load
Shadow bytes around the buggy address:
  0x7bf653f0e500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7bf653f0e580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7bf653f0e600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7bf653f0e680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7bf653f0e700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x7bf653f0e780: 00 00 00 00 02 fc[fc]fc fc fc fc fc fc fa fa fa
  0x7bf653f0e800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7bf653f0e880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7bf653f0e900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7bf653f0e980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7bf653f0ea00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==625312==ABORTING
ERROR Error: Command failed: /workspaces/gclient/src/out/Testing/electron index.js

Additional information

It appears potentially specific to the location of the main key in package.json - so far I'm consistently able to reproduce this with cookie-signature which has its main entry at the bottom. I'm not yet sure what other main entries could cause this, and have confirmed that moving main nearer to the top fixes the crash.

Refs #50322 cc @anonrig @lemire

[lemire]

@codebytere

Thanks for the report. I will be investigating this morning.

cc @anonrig

[codebytere]

@lemire you might already know this, but if not, ASAN appears broken in main right now as a result of a V8 bump from 3ish months ago (#54447). ASAN still works in Electron's GN build of Node.js though, and we don't patch anything to do with this area of the codebase, so I used our build's executable to repro. Let me know if you need any other info, and appreciate your investigating!

[lemire]

@codebytere I am somewhat puzzled by the trace. You see this line...

    #4 0x5891de703a34 in simdjson::haswell::dom_parser_implementation::parse_string(unsigned char const*, unsigned char*, bool) const third_party/electron_node/deps/simdjson/simdjson.cpp:20193:10

???

It indicates that the system detected an Intel Haswell-like CPU (so with AVX2).

But then you indicate the platform as being...

Darwin MacBookPro.fritz.box 24.1.0 Darwin Kernel Version 24.1.0: Thu Oct 10 21:03:15 PDT 2024; root:xnu-11215.41.3~2/RELEASE_ARM64_T6000 arm64

... which indicates an ARM-based system.

Is this running under rosetta?

[lemire]

@codebytere Can you specify the precise node version? (E.g., commit hash.)

The stack trace indicates the following as the starting point...

 third_party/electron_node/src/node_modules.cc:162:17

But that does not match the content of that line:

node/src/node_modules.cc

Line 162 in 4354a1d

return throw_invalid_package_config();

[codebytere]

@lemire you're right, my apologies - the stacktrace is from a Linux x64 VM. I've corrected it now, and the specific line causing the crash is:

node/src/node_modules.cc

Line 159 in 317d245

USE(value.get_string(package_config.main));

The reason it said L162 is because I added some logging locally:

    fprintf(stderr, "key: %s\n", key.raw());
    fprintf(stderr, "path: %s\n", path.data());

to help verify the contents of the package.json causing the crash, which is how i determined it was the cookie-signature package. The commit i've checked out is 4631be0.

[lemire]

@codebytere That's useful.

So tracing it back, we have this JSON file...

{
  "name": "cookie-signature",
  "version": "1.2.1",
  "description": "Sign and unsign cookies",
  "keywords": ["cookie", "sign", "unsign"],
  "author": "TJ Holowaychuk <tj@learnboost.com>",
  "license": "MIT",
  "repository": {
    "type": "git",
    "url": "https://github.com/visionmedia/node-cookie-signature.git"
  },
  "dependencies": {},
  "engines": {
    "node": ">=6.6.0"
  },
  "devDependencies": {
    "mocha": "*",
    "should": "*"
  },
  "scripts": {
    "test": "mocha --require should --reporter spec"
  },
  "main": "index"
}

And there is an issue where we try to read "index" (at the end) and somehow read too many bytes....

[codebytere]

I also played around a bit with key order and it looks like as long as main isn't the absolute last item it won't crash - the order of other keys before it doesn't seem to matter

[lemire]

It is not specific to AVX2 or any such thing.

[lemire]

Oh.

I understand. I did not know about container overflows, but this is done deliberately:

https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow

[lemire]

To be clear, this is a false positive.

[lemire]

Please see #55591