Skip to content

Commit e65c9ec

Browse files
gireeshpunathilMylesBorins
authored andcommitted
http: assert parser.consume argument's type
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Alexey Orlenko <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
1 parent 86497f1 commit e65c9ec

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

src/node_http_parser.cc

+1
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ class Parser : public AsyncWrap {
472472
static void Consume(const FunctionCallbackInfo<Value>& args) {
473473
Parser* parser;
474474
ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
475+
CHECK(args[0]->IsExternal());
475476
Local<External> stream_obj = args[0].As<External>();
476477
StreamBase* stream = static_cast<StreamBase*>(stream_obj->Value());
477478
CHECK_NE(stream, nullptr);
+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const http = require('http');
5+
const spawn = require('child_process').spawn;
6+
7+
if (process.argv[2] === 'child') {
8+
const server = http.createServer(common.mustCall((req, res) => {
9+
res.end('hello');
10+
}));
11+
12+
server.listen(0, common.mustCall((s) => {
13+
const rr = http.get(
14+
{ port: server.address().port },
15+
common.mustCall((d) => {
16+
// This bad input (0) should abort the parser and the process
17+
rr.parser.consume(0);
18+
server.close();
19+
}));
20+
}));
21+
} else {
22+
const child = spawn(process.execPath, [__filename, 'child'],
23+
{ stdio: 'inherit' });
24+
child.on('exit', common.mustCall((code, signal) => {
25+
assert(common.nodeProcessAborted(code, signal),
26+
'process should have aborted, but did not');
27+
}));
28+
}

0 commit comments

Comments
 (0)