From efab7847c682d2e3d63223e8724d64480a7f8aec Mon Sep 17 00:00:00 2001 From: Gireesh Punathil Date: Sun, 9 Apr 2017 15:57:19 +0530 Subject: [PATCH] http: assert parser.consume argument's type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/12288 Fixes: https://github.com/nodejs/node/issues/12178 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Alexey Orlenko Reviewed-By: James M Snell Reviewed-By: Santiago Gimeno Reviewed-By: Tobias Nießen Reviewed-By: Refael Ackermann --- src/node_http_parser.cc | 1 + test/abort/test-http-parser-consume.js | 28 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 test/abort/test-http-parser-consume.js diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 40e106cd46fd3c..a339043bdca5bd 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -496,6 +496,7 @@ class Parser : public AsyncWrap { static void Consume(const FunctionCallbackInfo& args) { Parser* parser; ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); + CHECK(args[0]->IsExternal()); Local stream_obj = args[0].As(); StreamBase* stream = static_cast(stream_obj->Value()); CHECK_NE(stream, nullptr); diff --git a/test/abort/test-http-parser-consume.js b/test/abort/test-http-parser-consume.js new file mode 100644 index 00000000000000..4a05a299a8e956 --- /dev/null +++ b/test/abort/test-http-parser-consume.js @@ -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'); + })); +}