Skip to content

Commit

Permalink
http: assert parser.consume argument's type
Browse files Browse the repository at this point in the history
Unchecked argument conversion in Parser::Consume crashes node
in an slightly undesirable manner - 'unreachable code' in parser.

Make sure we validate the incoming type at the earliest point.

PR-URL: #12288
Fixes: #12178
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
gireeshpunathil authored and MylesBorins committed Sep 19, 2017
1 parent 86497f1 commit e65c9ec
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ class Parser : public AsyncWrap {
static void Consume(const FunctionCallbackInfo<Value>& args) {
Parser* parser;
ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
CHECK(args[0]->IsExternal());
Local<External> stream_obj = args[0].As<External>();
StreamBase* stream = static_cast<StreamBase*>(stream_obj->Value());
CHECK_NE(stream, nullptr);
Expand Down
28 changes: 28 additions & 0 deletions test/abort/test-http-parser-consume.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');
const spawn = require('child_process').spawn;

if (process.argv[2] === 'child') {
const server = http.createServer(common.mustCall((req, res) => {
res.end('hello');
}));

server.listen(0, common.mustCall((s) => {
const rr = http.get(
{ port: server.address().port },
common.mustCall((d) => {
// This bad input (0) should abort the parser and the process
rr.parser.consume(0);
server.close();
}));
}));
} else {
const child = spawn(process.execPath, [__filename, 'child'],
{ stdio: 'inherit' });
child.on('exit', common.mustCall((code, signal) => {
assert(common.nodeProcessAborted(code, signal),
'process should have aborted, but did not');
}));
}

0 comments on commit e65c9ec

Please sign in to comment.