summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Noordhuis <info@bnoordhuis.nl>2014-08-23 00:57:45 +0200
committerTrevor Norris <trev.norris@gmail.com>2014-09-05 09:34:37 -0700
commit150d6f124942617428c36728c023341c83166449 (patch)
tree68cdfa4a70802f7decc986ee35e4bc9986a362fb
parent8e6706ea95c81534357c84237f561a0a8073c38e (diff)
downloadnodejs-150d6f124942617428c36728c023341c83166449.tar.gz
nodejs-150d6f124942617428c36728c023341c83166449.tar.bz2
nodejs-150d6f124942617428c36728c023341c83166449.zip
lib: http: poison parser references after freeing
Make it a little harder to slip in use-after-free bugs by nulling out references to the parser object after handing it off to freeParser(). Reviewed-by: Trevor Norris <trev.norris@gmail.com>
-rw-r--r--lib/_http_client.js12
-rw-r--r--lib/_http_common.js5
-rw-r--r--lib/_http_server.js8
3 files changed, 15 insertions, 10 deletions
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 8582e727b..929be3f2a 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -257,7 +257,7 @@ function socketCloseListener() {
if (parser) {
parser.finish();
- freeParser(parser, req);
+ freeParser(parser, req, socket);
}
}
@@ -276,7 +276,7 @@ function socketErrorListener(err) {
if (parser) {
parser.finish();
- freeParser(parser, req);
+ freeParser(parser, req, socket);
}
socket.destroy();
}
@@ -294,7 +294,7 @@ function socketOnEnd() {
}
if (parser) {
parser.finish();
- freeParser(parser, req);
+ freeParser(parser, req, socket);
}
socket.destroy();
}
@@ -309,7 +309,7 @@ function socketOnData(d) {
var ret = parser.execute(d);
if (ret instanceof Error) {
debug('parse error');
- freeParser(parser, req);
+ freeParser(parser, req, socket);
socket.destroy();
req.emit('error', ret);
req.socket._hadError = true;
@@ -344,7 +344,7 @@ function socketOnData(d) {
// Got Upgrade header or CONNECT method, but have no handler.
socket.destroy();
}
- freeParser(parser, req);
+ freeParser(parser, req, socket);
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is 100 (Continue), the server will
// send a final response after this client sends a request
@@ -352,7 +352,7 @@ function socketOnData(d) {
parser.incoming.statusCode !== 100) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
- freeParser(parser, req);
+ freeParser(parser, req, socket);
}
}
diff --git a/lib/_http_common.js b/lib/_http_common.js
index f3d9d4a2e..7994cdd98 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -192,7 +192,7 @@ exports.parsers = parsers;
// up by doing `parser.data = {}`, which should
// be done in FreeList.free. `parsers.free(parser)`
// should be all that is needed.
-function freeParser(parser, req) {
+function freeParser(parser, req, socket) {
if (parser) {
parser._headers = [];
parser.onIncoming = null;
@@ -207,6 +207,9 @@ function freeParser(parser, req) {
if (req) {
req.parser = null;
}
+ if (socket) {
+ socket.parser = null;
+ }
}
exports.freeParser = freeParser;
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 83cab6908..787fc27bb 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -285,8 +285,9 @@ function connectionListener(socket) {
function serverSocketCloseListener() {
debug('server socket close');
// mark this parser as reusable
- if (this.parser)
- freeParser(this.parser);
+ if (this.parser) {
+ freeParser(this.parser, null, this);
+ }
abortIncoming();
}
@@ -353,7 +354,8 @@ function connectionListener(socket) {
socket.removeListener('end', socketOnEnd);
socket.removeListener('close', serverSocketCloseListener);
parser.finish();
- freeParser(parser, req);
+ freeParser(parser, req, null);
+ parser = null;
var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (EventEmitter.listenerCount(self, eventName) > 0) {