Skip to content
Permalink
Browse files

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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
addaleax authored and BethGriggs committed Aug 10, 2019
1 parent ac28a62 commit 10f05b65c4c84c21e203271301dc5187253c66dc
Showing with 37 additions and 5 deletions.
  1. +11 −5 src/node_http2.cc
  2. +1 −0 src/node_revert.h
  3. +25 −0 test/parallel/test-http2-zero-length-header.js
@@ -1328,6 +1328,8 @@ inline 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;

Local<String> name_str;
Local<String> value_str;
@@ -2021,6 +2023,7 @@ Http2Stream::~Http2Stream() {
if (session_ == nullptr)
return;
DEBUG_HTTP2STREAM(this, "tearing down stream");
session_->DecrementCurrentSessionMemory(current_headers_length_);
session_->RemoveStream(this);
session_ = nullptr;

@@ -2032,6 +2035,7 @@ Http2Stream::~Http2Stream() {
void Http2Stream::StartHeaders(nghttp2_headers_category category) {
DEBUG_HTTP2STREAM2(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;
@@ -2323,10 +2327,6 @@ inline int Http2Stream::DoWrite(WriteWrap* req_wrap,
return 0;
}

inline size_t GetBufferLength(nghttp2_rcbuf* buf) {
return nghttp2_rcbuf_get_buf(buf).len;
}

// Ads a header to the Http2Stream. Note that the header name and value are
// provided using a buffer structure provided by nghttp2 that allows us to
// avoid unnecessary memcpy's. Those buffers are ref counted. The ref count
@@ -2338,7 +2338,12 @@ inline bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
CHECK(!this->IsDestroyed());
if (this->statistics_.first_header == 0)
this->statistics_.first_header = uv_hrtime();
size_t length = GetBufferLength(name) + GetBufferLength(value) + 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) ||
@@ -2354,6 +2359,7 @@ inline bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
nghttp2_rcbuf_incref(name);
nghttp2_rcbuf_incref(value);
current_headers_length_ += length;
session_->IncrementCurrentSessionMemory(length);
return true;
}

@@ -18,6 +18,7 @@ namespace node {
#define SECURITY_REVERSIONS(XX) \
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting") \
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.
@@ -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 10f05b6

Please sign in to comment.
You can’t perform that action at this time.