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

HTTPParser crashes the node process when given bad input #16552

Closed
maxnordlund opened this issue Oct 27, 2017 · 3 comments
Closed

HTTPParser crashes the node process when given bad input #16552

maxnordlund opened this issue Oct 27, 2017 · 3 comments
Labels
http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. http Issues or PRs related to the http subsystem.

Comments

@maxnordlund
Copy link

maxnordlund commented Oct 27, 2017

  • Version:
    v8.7.0
  • Platform:
    Darwin Maxs-MacBook-Pro.local 16.7.0 Darwin Kernel Version 16.7.0: Thu Jun 15 17:36:27 PDT 2017; root:xnu-3789.70.16~2/RELEASE_X86_64 x86_64
  • Subsystem:
    HTTPParser

I noticed that the HTTPParser is very sensitive, which is fine, but crashing the entire process is not. I would expect a normal exception to be thrown instead.

I'm sure there's a way to exploit this even though I believe HTTPParser is just used internally.

$ node
> const { HTTPParser } = process.binding('http_parser');
undefined
> HTTPParser()
/usr/local/Cellar/node/8.7.0/bin/node[33281]: ../src/async-wrap.cc:582:node::AsyncWrap::AsyncWrap(node::Environment *, Local<v8::Object>, node::AsyncWrap::ProviderType, double): Assertion `(object->InternalFieldCount()) >= (1)' failed.
 1: node::Abort() [/usr/local/bin/node]
 2: node::(anonymous namespace)::DomainEnter(node::Environment*, v8::Local<v8::Object>) [/usr/local/bin/node]
 3: node::AsyncWrap::AsyncWrap(node::Environment*, v8::Local<v8::Object>, node::AsyncWrap::ProviderType, double) [/usr/local/bin/node]
 4: node::(anonymous namespace)::Parser::New(v8::FunctionCallbackInfo<v8::Value> const&) [/usr/local/bin/node]
 5: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [/usr/local/bin/node]
 6: 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) [/usr/local/bin/node]
 7: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/bin/node]
 8: 0x75089c0463d
fish: 'node' terminated by signal SIGABRT (Abort)
@apapirovski apapirovski added http Issues or PRs related to the http subsystem. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. labels Oct 27, 2017
@maclover7
Copy link
Contributor

@maxnordlund I believe that everything under process.binding (including http_parser) is internal Node API, and is not meant to be called by userland modules

@bnoordhuis
Copy link
Member

@maclover7 is right. process.binding() is in the category of "if it breaks, you get to keep both pieces."

@maxnordlund
Copy link
Author

I do know that, I was more concerned about the possibility of an security exploit here. Hopefully that internal code doesn't trigger this, but I prefer to be sure rather then hopeful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants