Browse Source

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>
v0.11.14-release
Ben Noordhuis 11 years ago
committed by Trevor Norris
parent
commit
150d6f1249
  1. 12
      lib/_http_client.js
  2. 5
      lib/_http_common.js
  3. 8
      lib/_http_server.js

12
lib/_http_client.js

@ -257,7 +257,7 @@ function socketCloseListener() {
if (parser) { if (parser) {
parser.finish(); parser.finish();
freeParser(parser, req); freeParser(parser, req, socket);
} }
} }
@ -276,7 +276,7 @@ function socketErrorListener(err) {
if (parser) { if (parser) {
parser.finish(); parser.finish();
freeParser(parser, req); freeParser(parser, req, socket);
} }
socket.destroy(); socket.destroy();
} }
@ -294,7 +294,7 @@ function socketOnEnd() {
} }
if (parser) { if (parser) {
parser.finish(); parser.finish();
freeParser(parser, req); freeParser(parser, req, socket);
} }
socket.destroy(); socket.destroy();
} }
@ -309,7 +309,7 @@ function socketOnData(d) {
var ret = parser.execute(d); var ret = parser.execute(d);
if (ret instanceof Error) { if (ret instanceof Error) {
debug('parse error'); debug('parse error');
freeParser(parser, req); freeParser(parser, req, socket);
socket.destroy(); socket.destroy();
req.emit('error', ret); req.emit('error', ret);
req.socket._hadError = true; req.socket._hadError = true;
@ -344,7 +344,7 @@ function socketOnData(d) {
// Got Upgrade header or CONNECT method, but have no handler. // Got Upgrade header or CONNECT method, but have no handler.
socket.destroy(); socket.destroy();
} }
freeParser(parser, req); freeParser(parser, req, socket);
} else if (parser.incoming && parser.incoming.complete && } else if (parser.incoming && parser.incoming.complete &&
// When the status code is 100 (Continue), the server will // When the status code is 100 (Continue), the server will
// send a final response after this client sends a request // send a final response after this client sends a request
@ -352,7 +352,7 @@ function socketOnData(d) {
parser.incoming.statusCode !== 100) { parser.incoming.statusCode !== 100) {
socket.removeListener('data', socketOnData); socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd); socket.removeListener('end', socketOnEnd);
freeParser(parser, req); freeParser(parser, req, socket);
} }
} }

5
lib/_http_common.js

@ -192,7 +192,7 @@ exports.parsers = parsers;
// up by doing `parser.data = {}`, which should // up by doing `parser.data = {}`, which should
// be done in FreeList.free. `parsers.free(parser)` // be done in FreeList.free. `parsers.free(parser)`
// should be all that is needed. // should be all that is needed.
function freeParser(parser, req) { function freeParser(parser, req, socket) {
if (parser) { if (parser) {
parser._headers = []; parser._headers = [];
parser.onIncoming = null; parser.onIncoming = null;
@ -207,6 +207,9 @@ function freeParser(parser, req) {
if (req) { if (req) {
req.parser = null; req.parser = null;
} }
if (socket) {
socket.parser = null;
}
} }
exports.freeParser = freeParser; exports.freeParser = freeParser;

8
lib/_http_server.js

@ -285,8 +285,9 @@ function connectionListener(socket) {
function serverSocketCloseListener() { function serverSocketCloseListener() {
debug('server socket close'); debug('server socket close');
// mark this parser as reusable // mark this parser as reusable
if (this.parser) if (this.parser) {
freeParser(this.parser); freeParser(this.parser, null, this);
}
abortIncoming(); abortIncoming();
} }
@ -353,7 +354,8 @@ function connectionListener(socket) {
socket.removeListener('end', socketOnEnd); socket.removeListener('end', socketOnEnd);
socket.removeListener('close', serverSocketCloseListener); socket.removeListener('close', serverSocketCloseListener);
parser.finish(); parser.finish();
freeParser(parser, req); freeParser(parser, req, null);
parser = null;
var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (EventEmitter.listenerCount(self, eventName) > 0) { if (EventEmitter.listenerCount(self, eventName) > 0) {

Loading…
Cancel
Save