http2: handle 0-length headers better
Ignore headers with 0-length names and track memory for headers the way we track it for other HTTP/2 session memory too. This is intended to mitigate CVE-2019-9516. PR-URL: https://github.com/nodejs/node/pull/29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
parent
a54af9e188
commit
b4cfa521b8
@ -1309,6 +1309,8 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
|
||||
return;
|
||||
|
||||
std::vector<nghttp2_header> headers(stream->move_headers());
|
||||
DecrementCurrentSessionMemory(stream->current_headers_length_);
|
||||
stream->current_headers_length_ = 0;
|
||||
|
||||
// The headers are passed in above as a queue of nghttp2_header structs.
|
||||
// The following converts that into a JS array with the structure:
|
||||
@ -1942,6 +1944,7 @@ Http2Stream::~Http2Stream() {
|
||||
if (session_ == nullptr)
|
||||
return;
|
||||
Debug(this, "tearing down stream");
|
||||
session_->DecrementCurrentSessionMemory(current_headers_length_);
|
||||
session_->RemoveStream(this);
|
||||
session_ = nullptr;
|
||||
}
|
||||
@ -1956,6 +1959,7 @@ std::string Http2Stream::diagnostic_name() const {
|
||||
void Http2Stream::StartHeaders(nghttp2_headers_category category) {
|
||||
Debug(this, "starting headers, category: %d", id_, category);
|
||||
CHECK(!this->IsDestroyed());
|
||||
session_->DecrementCurrentSessionMemory(current_headers_length_);
|
||||
current_headers_length_ = 0;
|
||||
current_headers_.clear();
|
||||
current_headers_category_ = category;
|
||||
@ -2225,8 +2229,12 @@ bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
|
||||
CHECK(!this->IsDestroyed());
|
||||
if (this->statistics_.first_header == 0)
|
||||
this->statistics_.first_header = uv_hrtime();
|
||||
size_t length = nghttp2_rcbuf_get_buf(name).len +
|
||||
nghttp2_rcbuf_get_buf(value).len + 32;
|
||||
size_t name_len = nghttp2_rcbuf_get_buf(name).len;
|
||||
if (name_len == 0 && !IsReverted(SECURITY_REVERT_CVE_2019_9516)) {
|
||||
return true; // Ignore headers with empty names.
|
||||
}
|
||||
size_t value_len = nghttp2_rcbuf_get_buf(value).len;
|
||||
size_t length = name_len + value_len + 32;
|
||||
// A header can only be added if we have not exceeded the maximum number
|
||||
// of headers and the session has memory available for it.
|
||||
if (!session_->IsAvailableSessionMemory(length) ||
|
||||
@ -2242,6 +2250,7 @@ bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
|
||||
nghttp2_rcbuf_incref(name);
|
||||
nghttp2_rcbuf_incref(value);
|
||||
current_headers_length_ += length;
|
||||
session_->IncrementCurrentSessionMemory(length);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -17,6 +17,7 @@ namespace node {
|
||||
|
||||
#define SECURITY_REVERSIONS(XX) \
|
||||
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
|
||||
XX(CVE_2019_9516, "CVE-2019-9516", "HTTP/2 0-Length Headers Leak") \
|
||||
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
|
||||
// TODO(addaleax): Remove all of the above before Node.js 13 as the comment
|
||||
// at the start of the file indicates.
|
||||
|
25
test/parallel/test-http2-zero-length-header.js
Normal file
25
test/parallel/test-http2-zero-length-header.js
Normal file
@ -0,0 +1,25 @@
|
||||
'use strict';
|
||||
const common = require('../common');
|
||||
if (!common.hasCrypto)
|
||||
common.skip('missing crypto');
|
||||
|
||||
const assert = require('assert');
|
||||
const http2 = require('http2');
|
||||
|
||||
const server = http2.createServer();
|
||||
server.on('stream', (stream, headers) => {
|
||||
assert.deepStrictEqual(headers, {
|
||||
':scheme': 'http',
|
||||
':authority': `localhost:${server.address().port}`,
|
||||
':method': 'GET',
|
||||
':path': '/',
|
||||
'bar': '',
|
||||
'__proto__': null
|
||||
});
|
||||
stream.session.destroy();
|
||||
server.close();
|
||||
});
|
||||
server.listen(0, common.mustCall(() => {
|
||||
const client = http2.connect(`http://localhost:${server.address().port}/`);
|
||||
client.request({ ':path': '/', '': 'foo', 'bar': '' }).end();
|
||||
}));
|
Loading…
x
Reference in New Issue
Block a user