Commit 5ec7d1e
http2: validate initialWindowSize per HTTP/2 spec
The HTTP/2 spec (RFC 7540) defines SETTINGS_INITIAL_WINDOW_SIZE
maximum as 2^31-1. Values above this must be treated as a
FLOW_CONTROL_ERROR. Previously, Node.js allowed values up to
2^32-1 which caused nghttp2_submit_settings() to return
NGHTTP2_ERR_INVALID_ARGUMENT, triggering an uncatchable
assertion failure and crashing the process.
This change adds proper validation to reject values >= 2^31
with a catchable RangeError before they reach nghttp2.
PR-URL: #61402
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Tim Perry <pimterry@gmail.com>1 parent 80feaca commit 5ec7d1e
3 files changed
Lines changed: 8 additions & 3 deletions
File tree
- lib/internal/http2
- test/parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -217,6 +217,7 @@ function debugSessionObj(session, message, ...args) { | |||
| 217 | 217 | ||
| 218 | 218 | const kMaxFrameSize = (2 ** 24) - 1; | |
| 219 | 219 | const kMaxInt = (2 ** 32) - 1; | |
| 220 | + const kMaxInitialWindowSize = (2 ** 31) - 1; // HTTP/2 spec maximum | ||
| 220 | 221 | const kMaxStreams = (2 ** 32) - 1; | |
| 221 | 222 | const kMaxALTSVC = (2 ** 14) - 2; | |
| 222 | 223 | ||
@@ -956,7 +957,7 @@ function pingCallback(cb) { | |||
| 956 | 957 | ||
| 957 | 958 | // Validates the values in a settings object. Specifically: | |
| 958 | 959 | // 1. headerTableSize must be a number in the range 0 <= n <= kMaxInt | |
| 959 | - // 2. initialWindowSize must be a number in the range 0 <= n <= kMaxInt | ||
| 960 | + // 2. initialWindowSize must be a number in the range 0 <= n <= 2^31-1 | ||
| 960 | 961 | // 3. maxFrameSize must be a number in the range 16384 <= n <= kMaxFrameSize | |
| 961 | 962 | // 4. maxConcurrentStreams must be a number in the range 0 <= n <= kMaxStreams | |
| 962 | 963 | // 5. maxHeaderListSize must be a number in the range 0 <= n <= kMaxInt | |
@@ -981,7 +982,7 @@ const validateSettings = hideStackFrames((settings) => { | |||
| 981 | 982 | 0, kMaxInt); | |
| 982 | 983 | assertWithinRange.withoutStackTrace('initialWindowSize', | |
| 983 | 984 | settings.initialWindowSize, | |
| 984 | - 0, kMaxInt); | ||
| 985 | + 0, kMaxInitialWindowSize); | ||
| 985 | 986 | assertWithinRange.withoutStackTrace('maxFrameSize', | |
| 986 | 987 | settings.maxFrameSize, | |
| 987 | 988 | 16384, kMaxFrameSize); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -20,7 +20,7 @@ assert.deepStrictEqual(val, check); | |||
| 20 | 20 | ['headerTableSize', 0], | |
| 21 | 21 | ['headerTableSize', 2 ** 32 - 1], | |
| 22 | 22 | ['initialWindowSize', 0], | |
| 23 | - ['initialWindowSize', 2 ** 32 - 1], | ||
| 23 | + ['initialWindowSize', 2 ** 31 - 1], // Max per HTTP/2 spec | ||
| 24 | 24 | ['maxFrameSize', 16384], | |
| 25 | 25 | ['maxFrameSize', 2 ** 24 - 1], | |
| 26 | 26 | ['maxConcurrentStreams', 0], | |
@@ -42,6 +42,8 @@ http2.getPackedSettings({ enablePush: false }); | |||
| 42 | 42 | ['headerTableSize', -1], | |
| 43 | 43 | ['headerTableSize', 2 ** 32], | |
| 44 | 44 | ['initialWindowSize', -1], | |
| 45 | + ['initialWindowSize', 2 ** 31], // Max per HTTP/2 spec is 2^31-1 | ||
| 46 | + ['initialWindowSize', 2 ** 32 - 1], // Regression test for nghttp2 crash | ||
| 45 | 47 | ['initialWindowSize', 2 ** 32], | |
| 46 | 48 | ['maxFrameSize', 16383], | |
| 47 | 49 | ['maxFrameSize', 2 ** 24], | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -133,6 +133,8 @@ server.listen( | |||
| 133 | 133 | ['headerTableSize', -1], | |
| 134 | 134 | ['headerTableSize', 2 ** 32], | |
| 135 | 135 | ['initialWindowSize', -1], | |
| 136 | + ['initialWindowSize', 2 ** 31], // Max per HTTP/2 spec is 2^31-1 | ||
| 137 | + ['initialWindowSize', 2 ** 32 - 1], // Regression test for nghttp2 crash | ||
| 136 | 138 | ['initialWindowSize', 2 ** 32], | |
| 137 | 139 | ['maxFrameSize', 16383], | |
| 138 | 140 | ['maxFrameSize', 2 ** 24], | |
0 commit comments