8000 http2: handle 0-length headers better · nodejs/node@10f05b6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 10f05b6

Browse files
addaleaxBethGriggs
addaleax
authored and
BethGriggs
committed
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>
1 parent ac28a62 commit 10f05b6

File tree

3 files changed

+37
-5
lines changed

3 files changed

+37
-5
lines changed

‎src/node_http2.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,6 +1328,8 @@ inline void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
13281328
return;
13291329

13301330
std::vector<nghttp2_header> headers(stream->move_headers());
1331+
DecrementCurrentSessionMemory(stream->current_headers_length_);
1332+
stream->current_headers_length_ = 0;
13311333

13321334
Local<String> name_str;
13331335
Local<String> value_str;
@@ -2021,6 +2023,7 @@ Http2Stream::~Http2Stream() {
20212023
if (session_ == nullptr)
20222024
return;
20232025
DEBUG_HTTP2STREAM(this, "tearing down stream");
2026+
session_->DecrementCurrentSessionMemory(current_headers_length_);
20242027
session_->RemoveStream(this);
20252028
session_ = nullptr;
20262029

@@ -2032,6 +2035,7 @@ Http2Stream::~Http2Stream() {
20322035
void Http2Stream::StartHeaders(nghttp2_headers_category category) {
20332036
DEBUG_HTTP2STREAM2(this, "starting headers, category: %d", id_, category);
20342037
CHECK(!this->IsDestroyed());
2038+
session_->DecrementCurrentSessionMemory(current_headers_length_);
20352039
current_headers_length_ = 0;
20362040
current_headers_.clear();
20372041
current_headers_category_ = category;
@@ -2323,10 +2327,6 @@ inline int Http2Stream::DoWrite(WriteWrap* req_wrap,
23232327
return 0;
23242328
}
23252329

2326-
inline size_t GetBufferLength(nghttp2_rcbuf* buf) {
2327-
return nghttp2_rcbuf_get_buf(buf).len;
2328-
}
2329-
23302330
// Ads a header to the Http2Stream. Note that the header name and value are
23312331
// provided using a buffer structure provided by nghttp2 that allows us to
23322332
// avoid unnecessary memcpy's. Those buffers are ref counted. The ref count
@@ -2338,7 +2338,12 @@ inline bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
23382338
CHECK(!this->IsDestroyed());
23392339
if (this->statistics_.first_header == 0)
23402340
this->statistics_.first_header = uv_hrtime();
2341-
size_t length = GetBufferLength(name) + GetBufferLength(value) + 32;
2341+
size_t name_len = nghttp2_rcbuf_get_buf(name).len;
2342+
if (name_len == 0 && !IsReverted(SECURITY_REVERT_CVE_2019_9516)) {
2343+
return true; // Ignore headers with empty names.
2344+
}
2345+
size_t value_len = nghttp2_rcbuf_get_buf(value).len;
2346+
size_t length = name_len + value_len + 32;
23422347
// A header can only be added if we have not exceeded the maximum number
23432348
// of headers and the session has memory available for it.
23442349
if (!session_->IsAvailableSessionMemory(length) ||
@@ -2354,6 +2359,7 @@ inline bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
23542359
nghttp2_rcbuf_incref(name);
23552360
nghttp2_rcbuf_incref(value);
23562361
current_headers_length_ += length;
2362+
session_->IncrementCurrentSessionMemory(length);
23572363
return true;
23582364
}
23592365

‎src/node_revert.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ namespace node {
1818
#define SECURITY_REVERSIONS(XX) \
1919
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting") \
2020
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
21+
XX(CVE_2019_9516, "CVE-2019-9516", "HTTP/2 0-Length Headers Leak") \
2122
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
2223
// TODO(addaleax): Remove all of the above before Node.js 13 as the comment
2324
// at the start of the file indicates.

‎test/parallel/test-http2-zero-length-header.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
const server = http2.createServer();
10+
server.on('stream', (stream, headers) => {
11+
assert.deepStrictEqual(headers, {
12+
':scheme': 'http',
13+
':authority': `localhost:${server.address().port}`,
14+
':method': 'GET',
15+
':path': '/',
16+
'bar': '',
17+
'__proto__': null
18+
});
19+
stream.session.destroy();
20+
server.close();
21+
});
22+
server.listen(0, common.mustCall(() => {
23+
const client = http2.connect(`http://localhost:${server.address().port}/`);
24+
client.request({ ':path': '/', '': 'foo', 'bar': '' }).end();
25+
}));

0 commit comments

Comments
 (0)
0