From 99943e189d7abd978cc7798bf768c4487920b365 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 25 Sep 2015 23:36:06 -0400 Subject: [PATCH] http: fix out-of-order 'finish' bug in pipelining Changes to `stream_base.cc` are required to support empty writes. Fixes CVE-2015-7384, https://github.com/nodejs/node/issues/3138 Fix: https://github.com/nodejs/node/issues/2639 PR-URL: https://github.com/nodejs/node/pull/3128 --- lib/_http_outgoing.js | 10 ++++---- src/stream_base.cc | 2 +- test/parallel/test-http-pipeline-regr-2639.js | 34 +++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-http-pipeline-regr-2639.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 0522ecb..0b725d5 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -133,12 +133,6 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) { encoding = null; } - if (data.length === 0) { - if (typeof callback === 'function') - process.nextTick(callback); - return true; - } - var connection = this.connection; if (connection && connection._httpMessage === this && @@ -158,6 +152,10 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) { this.output = []; this.outputEncodings = []; this.outputCallbacks = []; + } else if (data.length === 0) { + if (typeof callback === 'function') + process.nextTick(callback); + return true; } // Directly write to socket. diff --git a/src/stream_base.cc b/src/stream_base.cc index 3139274..27ae0fe 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -154,7 +154,7 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { // Write string offset = ROUND_UP(offset, WriteWrap::kAlignSize); - CHECK_LT(offset, storage_size); + CHECK_LE(offset, storage_size); char* str_storage = req_wrap->Extra(offset); size_t str_size = storage_size - offset; diff --git a/test/parallel/test-http-pipeline-regr-2639.js b/test/parallel/test-http-pipeline-regr-2639.js new file mode 100644 index 0000000..f1f75bf --- /dev/null +++ b/test/parallel/test-http-pipeline-regr-2639.js @@ -0,0 +1,34 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +const COUNT = 10; + +var received = 0; + +var server = http.createServer(function(req, res) { + // Close the server, we have only one TCP connection anyway + if (received++ === 0) + server.close(); + + res.writeHead(200); + res.write('data'); + + setTimeout(function() { + res.end(); + }, (Math.random() * 100) | 0); +}).listen(common.PORT, function() { + const s = net.connect(common.PORT); + + var big = ''; + for (var i = 0; i < COUNT; i++) + big += 'GET / HTTP/1.0\r\n\r\n'; + s.write(big); + s.resume(); +}); + +process.on('exit', function() { + assert.equal(received, COUNT); +}); -- 2.7.4