Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

null character as first character of path component crashes node #13787

Closed
zimbabao opened this issue Jun 19, 2017 · 9 comments
Closed

null character as first character of path component crashes node #13787

zimbabao opened this issue Jun 19, 2017 · 9 comments
Labels
module Issues and PRs related to the module subsystem.

Comments

@zimbabao
Copy link
Contributor

zimbabao commented Jun 19, 2017

  • Version: 9.00-pre, 8.1.0
  • Platform: MacOS X, linux
  • Subsystem: module

null character as first character of path component crashes node

require('\u0000abc')

causes node to crash.

This bug was found using fuzzer american fuzzy lop

Edit: Corrected \u0001 to \u0000

@TimothyGu
Copy link
Member

I can't reproduce this:

$ git log --oneline  -1
6e69421e53 doc: `path.relative` uses `cwd`
$ ./node -e "require('\u0001abc')"
module.js:487
    throw err;
    ^

Error: Cannot find module '�abc'
    at Function.Module._resolveFilename (module.js:485:15)
    at Function.Module._load (module.js:437:25)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at [eval]:1:1
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:569:30)
    at evalScript (bootstrap_node.js:432:27)
$ uname -srvmo
Linux 4.9.0-3-amd64 #1 SMP Debian 4.9.30-2 (2017-06-12) x86_64 GNU/Linux

@bnoordhuis
Copy link
Member

I suspect it should be \u0000, not \u0001?

$ ./out/Release/node -p 'require("\u0000abc")'
/Users/bnoordhuis/src/v1.x/out/Release/node[1089]: ../src/node_file.cc:527:void node::InternalModuleReadFile(const FunctionCallbackInfo<v8::Value> &): Assertion `(numchars) >= (0)' failed.
 1: node::Abort() [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 2: node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, char const*, int, v8::Local<v8::Value>*, double, double) [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 3: node::InternalModuleReadFile(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 5: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 6: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/bnoordhuis/src/v1.x/./out/Release/node]
 7: 0x1c91bfd843fd
 8: 0x1c91bfe78752
 9: 0x1c91bfe3be7c
Received signal 6

==== C stack trace ===============================

 [0x000100c3ea54]
 [0x7fffaa2c9b3a]
 [0x000100000001]
 [0x7fffaa14e420]
 [0x000100acae46]
 [0x000100ac9dfc]
 [0x000100af57a9]
 [0x000100220bc0]
 [0x00010029c4f2]
 [0x00010029b9cf]
 [0x1c91bfd843fd]
 [0x1c91bfe78752]
 [0x1c91bfe3be7c]
[end of stack trace]
Abort trap: 6

@vsemozhetbyt vsemozhetbyt added the module Issues and PRs related to the module subsystem. label Jun 19, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 19, 2017

I cannot reproduce on Windows 7 x64 with these versions:

(click me):
>node.4.8.3.v8-4.5.exe -p "require('\u0000abc')"
module.js:327
    throw err;
    ^
Error: Cannot find module '

>node.6.11.0.v8-5.1.exe -p "require('\u0000abc')"
module.js:471
    throw err;
    ^
Error: Cannot find module '

>node.8.0.0.v8-6.0.20170529.canary-base.exe -p "require('\u0000abc')"
module.js:500
    throw err;
    ^
Error: Cannot find module '

>node.8.1.1.v8-5.8.20170613.exe -p "require('\u0000abc')"
module.js:487
    throw err;
    ^
Error: Cannot find module '

>node.9.0.0.v8-5.9.20170618.nightly.exe -p "require('\u0000abc')"
module.js:487
    throw err;
    ^
Error: Cannot find module '

>node.9.0.0.v8-6.1.20170618.v8-canary.exe -p "require('\u0000abc')"
module.js:487
    throw err;
    ^
Error: Cannot find module '

@Ginden
Copy link

Ginden commented Jun 19, 2017

I can reproduce on Node v6.10.3 on OS X.

$ screenfetch
...
OS: 64bit Mac OS X 10.12.5 16F73
Kernel: x86_64 Darwin 16.6.0
...
$ node -e "require('\u0000abc')"
/Users/michal/.nvm/versions/node/v6.10.3/bin/node[5078]: ../src/node_file.cc:598:void node::InternalModuleReadFile(const FunctionCallbackInfo<v8::Value> &): Assertion `(numchars) >= (0)' failed.
 1: node::Abort() [/Users/michal/.nvm/versions/node/v6.10.3/bin/node]

zimbabao added a commit to zimbabao/node that referenced this issue Jun 19, 2017
Null char as the first char as the path component
of first argument of require causes a node crash.
Ignoring null and all chars after that in require path.

Fixes: nodejs#13787
zimbabao added a commit to zimbabao/node that referenced this issue Jun 19, 2017
Null char as the first char as the path component
of first argument of require causes a node crash.
Ignoring null and all chars after that in require path.

Fixes: nodejs#13787
@charmander
Copy link
Contributor

@vsemozhetbyt How about this?

$ node -e 'require("./\0abc")'

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 19, 2017

@charmander I've reverted the quotes as node sees your variant as a string:

(click me):
>node.4.8.3.v8-4.5.exe  -e "require('./\0abc')"
Assertion failed: (numchars) >= (0), file src\node_file.cc, line 492

>node.6.11.0.v8-5.1.exe  -e "require('./\0abc')"
 node.6.11.0.v8-5.1.exe: src\node_file.cc:598: Assertion `(numchars) >= (0)' failed.

>node.8.0.0.v8-6.0.20170529.canary-base.exe  -e "require('./\0abc')"
 node.8.0.0.v8-6.0.20170529.canary-base.exe: src\node_file.cc:523: Assertion `(numchars) >= (0)' failed.

>node.8.1.1.v8-5.8.20170613.exe  -e "require('./\0abc')"
 node.8.1.1.v8-5.8.20170613.exe: src\node_file.cc:527: Assertion `(numchars) >= (0)' failed.

>node.9.0.0.v8-5.9.20170618.nightly.exe  -e "require('./\0abc')"
 node.9.0.0.v8-5.9.20170618.nightly.exe: src\node_file.cc:527: Assertion `(numchars) >= (0)' failed.

>node.9.0.0.v8-6.1.20170618.v8-canary.exe  -e "require('./\0abc')"
 node.9.0.0.v8-6.1.20170618.v8-canary.exe: src\node_file.cc:527: Assertion `(numchars) >= (0)' failed.

zimbabao added a commit to zimbabao/node that referenced this issue Jun 20, 2017
Null char as the first char as the path component
of first argument of require causes a node crash.
Ignoring null and all chars after that in require path.

Fixes: nodejs#13787
zimbabao added a commit to zimbabao/node that referenced this issue Jun 20, 2017
Null char as the first char as the path component
of first argument of require causes a node crash.
Ignoring null and all chars after that in require path.

Fixes: nodejs#13787
@bnoordhuis
Copy link
Member

For anyone stumbling upon this issue, in-progress pull request here: #13788

@zimbabao
Copy link
Contributor Author

zimbabao commented Jul 3, 2017

There is one more pull requests is here(#8277)

@hubbertj
Copy link

Still present in v8.4.0!
error:
node -e "require('\u0000abc')"
/Users/Jay/.nvm/versions/node/v8.4.0/bin/node[22326]: ../src/node_file.cc:530:void node::InternalModuleReadFile(const FunctionCallbackInfov8::Value &): Assertion `(numchars) >= (0)' failed.
1: node::Abort() [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
2: node::MakeCallback(v8::Isolate*, v8::Localv8::Object, char const*, int, v8::Localv8::Value, node::async_context) [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
3: node::InternalModuleReadFile(v8::FunctionCallbackInfov8::Value const&) [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
4: v8::internal::FunctionCallbackArguments::Call(void (
)(v8::FunctionCallbackInfov8::Value const&)) [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
5: v8::internal::MaybeHandlev8::internal::Object v8::internal::(anonymous namespace)::HandleApiCallHelper(v8::internal::Isolate*, v8::internal::Handlev8::internal::HeapObject, v8::internal::Handlev8::internal::HeapObject, v8::internal::Handlev8::internal::FunctionTemplateInfo, v8::internal::Handlev8::internal::Object, v8::internal::BuiltinArguments) [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
6: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/Jay/.nvm/versions/node/v8.4.0/bin/node]
7: 0x383b32c840dd
[1] 22326 abort node -e "require('\u0000abc')"

nvm use v6.0.0
Now using node v6.0.0 (npm v3.8.6)
node -e "require('\u0000abc')"
Assertion failed: ((numchars) >= (0)), function InternalModuleReadFile, file ../src/node_file.cc, line 553.
[1] 23589 abort node -e "require('\u0000abc')"

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Aug 16, 2017
Throw an exception when the path contains nul bytes, don't abort.

Fixes: nodejs#13787
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Throw an exception when the path contains nul bytes, don't abort.

Fixes: #13787
PR-URL: #14854
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Throw an exception when the path contains nul bytes, don't abort.

Fixes: #13787
PR-URL: #14854
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 20, 2017
Throw an exception when the path contains nul bytes, don't abort.

Fixes: #13787
PR-URL: #14854
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants