diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2014-08-23 00:57:45 +0200 |
---|---|---|
committer | Trevor Norris <trev.norris@gmail.com> | 2014-09-05 09:34:37 -0700 |
commit | 150d6f124942617428c36728c023341c83166449 (patch) | |
tree | 68cdfa4a70802f7decc986ee35e4bc9986a362fb | |
parent | 8e6706ea95c81534357c84237f561a0a8073c38e (diff) | |
download | nodejs-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.js | 12 | ||||
-rw-r--r-- | lib/_http_common.js | 5 | ||||
-rw-r--r-- | lib/_http_server.js | 8 |
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) { |