Skip to content

Commit

Permalink
http2: handle 0-length headers better
Browse files Browse the repository at this point in the history
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: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and targos committed Aug 15, 2019
1 parent a21a1c0 commit 4f10ac3
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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) ||
Expand All @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions src/node_revert.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-http2-zero-length-header.js
Original file line number Diff line number Diff line change
@@ -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();
}));

0 comments on commit 4f10ac3

Please sign in to comment.