Repository navigation
Node crashes on new URL #48254
Description
Activity
Is this somehow platform-specific? I can't reproduce on Windows (I tried with all most recent node versions, and v19.8.1).
Ok, just tried v19.8.1 on Debian 5.10.158-2 (2022-12-13) x86_64 GNU/Linux, cannot reproduce it there either.
The relevant difference is that in Debian, I'm seeing /usr/include/c++/10/string_view, whereas in Arch Linux, it's version 12.2.1. Perhaps that's why.
cc @nodejs/url @lemire
I can't reproduce on my arch linux machine. Also tried running under
valgrind, and nothing showed up.$ ./node -e "new url('/sitelet?url=https%3A%2F%2Fgithub.com%2Fnodejs%2Fnode%2Fissues%2F%27%2C%2520%27localhost%3A80')" node:internal/url:556 throw new ERR_INVALID_url(/sitelet?url=https%3A%2F%2Fgithub.com%2Fnodejs%2Fnode%2Fissues%2Finput); ^ TypeError [ERR_INVALID_URL]: Invalid URL at new NodeError (node:internal/errors:399:5) at new URL (node:internal/url:556:13) at [eval]:1:1 at Script.runInThisContext (node:vm:128:12) at Object.runInThisContext (node:vm:306:38) at node:internal/process/execution:83:21 at [eval]-wrapper:6:24 at runScript (node:internal/process/execution:82:62) at evalScript (node:internal/process/execution:104:10) at node:internal/main/eval_string:50:3 { input: '', code: 'ERR_INVALID_URL' } Node.js v19.8.1 $ /usr/lib/libc.so.6 GNU C Library (GNU libc) stable release version 2.37. Copyright (C) 2023 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compiled by GNU CC version 13.1.1 20230429. libc ABIs: UNIQUE IFUNC ABSOLUTE Minimum supported kernel: 4.4.0 For bug reporting instructions, please see: <https://bugs.archlinux.org/>.- addedwhatwg-urlIssues and PRs related to the WHATWG URL implementation.Issues and PRs related to the WHATWG URL implementation.
on May 31, 2023 Perhaps you could post a backtrace from gdb or valgrind output?
I think we're hitting the assert added in gcc-mirror/gcc@3eefb30 ?
I'm not comfortable with C++ but I guess it uncovers a bug somewhere in Node.js or Ada code ?
I'm not sure how to do the gdb debugging?
I tried the inspect, but not sure how helpful that could still be:
break in node:internal/url:551 549 } 550 >551 const isValid = parse(input, 552 base, 553 this.#onParseComplete); < /usr/include/c++/12.2.1/string_view:239: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.I reviewed the code, if the relevant code is indeed in
BindingData::Parse()ofnode/src/node_url.cc, then given:- the issue does not occur when
args[0]is non-empty - and the issue is a failed assert in
basic_string_view
The error must occur at line 215 of
node/src/node_url.cc:auto out = ada::parse<ada::url_aggregator>(input.ToStringView(), base_pointer);EDIT: Missed something.
- the issue does not occur when
@jetibest can you try it with the last node.js version?
I'm not sure how to do the gdb debugging?
Create a script which contains the offending code (e.g.,
new url('/sitelet?url=https%3A%2F%2Fgithub.com%2Fnodejs%2Fnode%2Fissues%2F%27%2C%2520%27localhost%3A80')). Then launch node in gdb, and executerun. Should something like this...$ gdb node > run myscript.jsNote that you can use lldb also.
Once it crashes, you can type
btwhile in gdb. It should provide a stack trace.I have created a test that should replicate this bug in ada at ada-url/ada#421 So far none of the CI runs have hit a problem.
It does not look like we can reproduce the problem in ada per se.
If the problem is in ada, my guess would be that we could fix it by upgrading ada in node 19.
Node.js 19 is no longer supported (except for critical or security fixes)
@targos If a certain user input can potentially lead to crashing Node, which cannot be caught as an exception as expected, then this could technically be exploited to crash the application (= DoS attack). It doesn't really matter to me personally, but wouldn't this classify it as a security bug?
@lemire Thank you for the mini-tutorial, see the results below:
I made a script at
/tmp/test.js:new url('/sitelet?url=https%3A%2F%2Fgithub.com%2Fnodejs%2Fnode%2Fissues%2F%27%2C%2520%27localhost%3A80')Then I ran
gdb node:GNU gdb (GDB) 13.1 Copyright (C) 2023 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from node... This GDB supports auto-downloading debuginfo from the following URLs: <https://debuginfod.archlinux.org> Enable debuginfod for this session? (y or [n]) y Debuginfod has been enabled. To make this setting permanent, add 'set debuginfod enabled on' to .gdbinit. Downloading separate debug info for /usr/bin/node (No debugging symbols found in node) (gdb) run /tmp/test.js Starting program: /usr/bin/node /tmp/test.js Downloading separate debug info for /lib64/ld-linux-x86-64.so.2 Downloading separate debug info for system-supplied DSO at 0x7ffff7fc8000 Downloading separate debug info for /usr/bin/../lib/libnode.so.111 Downloading separate debug info for /usr/bin/../lib/libc.so.6 [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib/libthread_db.so.1". Downloading separate debug info for /usr/lib/libz.so.1 Downloading separate debug info for /usr/lib/libuv.so.1 Downloading separate debug info for /usr/lib/libbrotlidec.so.1 Downloading separate debug info for /usr/lib/libbrotlienc.so.1 Downloading separate debug info for /usr/lib/libcares.so.2 Downloading separate debug info for /usr/lib/libnghttp2.so.14 Downloading separate debug info for /usr/lib/libcrypto.so.3 Downloading separate debug info for /usr/lib/libssl.so.3 Downloading separate debug info for /usr/lib/libicui18n.so.72 Downloading separate debug info for /usr/lib/libicuuc.so.72 Downloading separate debug info for /usr/lib/libm.so.6 Downloading separate debug info for /usr/lib/libbrotlicommon.so.1 Downloading separate debug info for /usr/lib/libicudata.so.72 [New Thread 0x7ffff17ff6c0 (LWP 2353)] [New Thread 0x7ffff0ffe6c0 (LWP 2354)] [New Thread 0x7fffebfff6c0 (LWP 2355)] [New Thread 0x7fffeb7fe6c0 (LWP 2356)] [New Thread 0x7fffeaffd6c0 (LWP 2357)] [New Thread 0x7ffff7fc16c0 (LWP 2358)] /usr/include/c++/12.2.1/string_view:239: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed. Thread 1 "node" received signal SIGABRT, Aborted. 0x00007ffff48a08ec in ?? () from /usr/bin/../lib/libc.so.6 (gdb) bt #0 0x00007ffff48a08ec in ?? () from /usr/bin/../lib/libc.so.6 #1 0x00007ffff4851ea8 in raise () from /usr/bin/../lib/libc.so.6 #2 0x00007ffff483b53d in abort () from /usr/bin/../lib/libc.so.6 #3 0x00007ffff36d30e2 in std::__glibcxx_assert_fail (file=<optimized out>, line=<optimized out>, function=<optimized out>, condition=<optimized out>) at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:60 #4 0x00007ffff58bf258 in ada::parser::parse_url(std::basic_string_view<char, std::char_traits<char> >, ada::url const*, ada::encoding_type) () from /usr/bin/../lib/libnode.so.111 #5 0x00007ffff58c1188 in ada::parse(std::basic_string_view<char, std::char_traits<char> >, ada::url const*, ada::encoding_type) () from /usr/bin/../lib/libnode.so.111 #6 0x00007ffff575bf82 in ?? () from /usr/bin/../lib/libnode.so.111 #7 0x00007ffff5abbf27 in ?? () from /usr/bin/../lib/libnode.so.111 #8 0x00007ffff5abcb56 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) () from /usr/bin/../lib/libnode.so.111 #9 0x00007ffff59594f9 in ?? () from /usr/bin/../lib/libnode.so.111 #10 0x00003578cd8815b1 in ?? () #11 0x00007fffffffd310 in ?? () #12 0x0000000000000032 in ?? () #13 0x00007fffffffd3e8 in ?? () #14 0x00007ffff58d16dc in ?? () from /usr/bin/../lib/libnode.so.111 #15 0x00003578cd8815b1 in ?? () #16 0x00003ef313b749b1 in ?? () #17 0x0000000800000000 in ?? () #18 0x00003578cd881681 in ?? () #19 0x0000037609384ea9 in ?? () #20 0x00003578cd8817c9 in ?? () #21 0x00000ee85e7d89c1 in ?? () #22 0x000029c1fb0f6d11 in ?? () #23 0x000029c1fb0f6d11 in ?? () #24 0x00000ee85e7d89c1 in ?? () #25 0x00003578cd8817c9 in ?? () #26 0x00003ef313b749b1 in ?? () #27 0x000010b1da06a691 in ?? () #28 0x000029c1fb0f6c81 in ?? () #29 0x00003578cd8815b1 in ?? () #30 0x00000ee85e7d89c1 in ?? () #31 0x00003578cd8817c9 in ?? () #32 0x0000008600000000 in ?? () #33 0x00000ee85e7d9891 in ?? () #34 0x0000000000000003 in ?? () #35 0x000010b1da056709 in ?? () #36 0x000010b1da057c81 in ?? () #37 0x00007fffffffd440 in ?? () #38 0x00007ffff58cf1df in ?? () from /usr/bin/../lib/libnode.so.111 #39 0x000029c1fb0f6c81 in ?? () #40 0x00003578cd8817c9 in ?? () #41 0x00000ee85e7d89c1 in ?? () #42 0x000029c1fb0f6c81 in ?? () #43 0x00003578cd881681 in ?? () #44 0x000010b1da056709 in ?? () #45 0x0000000300000000 in ?? () #46 0x0000037609384f11 in ?? () #47 0x000000000000002e in ?? () #48 0x00007fffffffd4b0 in ?? () #49 0x00007ffff5a06e86 in ?? () from /usr/bin/../lib/libnode.so.111 #50 0x0000000000000000 in ?? ()If a certain user input can potentially lead to crashing Node, which cannot be caught as an exception as expected, then this could technically be exploited to crash the application (= DoS attack).
It is not an exception. It seems that Node.js is compiled without NDEBUG which means that asserts translate into a terminate. In this sense, the crash is deliberate. That is, Node is not meant to recover from the assert.
Just upgraded NodeJS to v20.2.0, unable to reproduce this issue there.
It is possible that earlier versions of ada had a bug that is being triggered under some conditions. If we think that there is an issue, the solution would be to update ada in Node.js 19 and issue a patch release. Node 19.8.1 uses ada 1.0.4. We are now at ada 2.5.0.
I should say that I could not reproduce the issue.
I think we're hitting the assert added in gcc-mirror/gcc@3eefb30 ?
Yes. It looks to me that the commit you refer to goes back to 2020 and is part of the releases 11.3 and later, 12.1 and later. We test ada with these compilers.
Reacted by Leonidov YevhenIf it's not reproducible / is fixed in v20.2.0, I suggest we close this issue. Node.js 19 goes EoL tomorrow.
Version
v19.8.1
Platform
Linux pc 6.2.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 18 Mar 2023 01:06:36 +0000 x86_64 GNU/Linux
Subsystem
url
What steps will reproduce the bug?
Running in Node:
How often does it reproduce? Is there a required condition?
There's no issue when the second argument is only a hostname, or a port. It only appears to lead to an issue when both the hostname and port is given in the second argument.
What is the expected behavior? Why is that the expected behavior?
An Error should be thrown with code
ERR_INVALID_URL:What do you see instead?
The error given in the steps to reproduce shows up, and Node crashes.
Additional information
No response