http2: header field valid checks

checks validity for request, writeHead, and setHeader methods PR-URL: #33193 Refs: #29829 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · May 30, 2020 · df31f71 · df31f71
1 parent 2c0a4fa
commit df31f71
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 2 deletions.
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -46,7 +46,13 @@ const {
   hideStackFrames
 } = require('internal/errors');
 const { validateString } = require('internal/validators');
-const { kSocket, kRequest, kProxySocket } = require('internal/http2/util');
+const {
+  kSocket,
+  kRequest,
+  kProxySocket,
+  assertValidPseudoHeader
+} = require('internal/http2/util');
+const { _checkIsHttpToken: checkIsHttpToken } = require('_http_common');
 
 const kBeginSend = Symbol('begin-send');
 const kState = Symbol('state');
@@ -590,6 +596,11 @@ class Http2ServerResponse extends Stream {
       return;
     }
 
+    if (name[0] === ':')
+      assertValidPseudoHeader(name);
+    else if (!checkIsHttpToken(name))
+      this.destroy(new ERR_INVALID_HTTP_TOKEN('Header name', name));
+
     this[kHeaders][name] = value;
   }
 

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -9,6 +9,7 @@ const {
   MathMin,
   ObjectAssign,
   ObjectCreate,
+  ObjectKeys,
   ObjectDefineProperty,
   ObjectPrototypeHasOwnProperty,
   Promise,
@@ -35,7 +36,10 @@ const tls = require('tls');
 const { URL } = require('url');
 const { setImmediate } = require('timers');
 
-const { kIncomingMessage } = require('_http_common');
+const {
+  kIncomingMessage,
+  _checkIsHttpToken: checkIsHttpToken
+} = require('_http_common');
 const { kServerResponse } = require('_http_server');
 const JSStreamSocket = require('internal/js_stream_socket');
 
@@ -88,6 +92,7 @@ const {
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_CALLBACK,
     ERR_INVALID_CHAR,
+    ERR_INVALID_HTTP_TOKEN,
     ERR_INVALID_OPT_VALUE,
     ERR_OUT_OF_RANGE,
     ERR_SOCKET_CLOSED
@@ -108,6 +113,7 @@ const { onServerStream,
 
 const {
   assertIsObject,
+  assertValidPseudoHeader,
   assertValidPseudoHeaderResponse,
   assertValidPseudoHeaderTrailer,
   assertWithinRange,
@@ -1602,6 +1608,15 @@ class ClientHttp2Session extends Http2Session {
 
     this[kUpdateTimer]();
 
+    if (headers !== null && headers !== undefined) {
+      for (const header of ObjectKeys(headers)) {
+        if (header[0] === ':') {
+          assertValidPseudoHeader(header);
+        } else if (header && !checkIsHttpToken(header))
+          this.destroy(new ERR_INVALID_HTTP_TOKEN('Header name', header));
+      }
+    }
+
     assertIsObject(headers, 'headers');
     assertIsObject(options, 'options');
 

diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
@@ -595,6 +595,7 @@ function sessionName(type) {
 
 module.exports = {
   assertIsObject,
+  assertValidPseudoHeader,
   assertValidPseudoHeaderResponse,
   assertValidPseudoHeaderTrailer,
   assertWithinRange,

diff --git a/test/parallel/test-http2-invalidheaderfields-client.js b/test/parallel/test-http2-invalidheaderfields-client.js
@@ -0,0 +1,55 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto) { common.skip('missing crypto'); }
+const assert = require('assert');
+const http2 = require('http2');
+
+const server1 = http2.createServer();
+
+server1.listen(0, common.mustCall(() => {
+  const session = http2.connect(`http://localhost:${server1.address().port}`);
+  // Check for req headers
+  session.request({ 'no underscore': 123 });
+  session.on('error', common.mustCall((e) => {
+    assert.strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN');
+    server1.close();
+  }));
+}));
+
+const server2 = http2.createServer(common.mustCall((req, res) => {
+  // check for setHeader
+  res.setHeader('x y z', 123);
+  res.end();
+}));
+
+server2.listen(0, common.mustCall(() => {
+  const session = http2.connect(`http://localhost:${server2.address().port}`);
+  const req = session.request();
+  req.on('error', common.mustCall((e) => {
+    assert.strictEqual(e.code, 'ERR_HTTP2_STREAM_ERROR');
+    session.close();
+    server2.close();
+  }));
+}));
+
+const server3 = http2.createServer(common.mustCall((req, res) => {
+  // check for writeHead
+  assert.throws(common.mustCall(() => {
+    res.writeHead(200, {
+      'an invalid header': 123
+    });
+  }), {
+    code: 'ERR_HTTP2_INVALID_STREAM'
+  });
+  res.end();
+}));
+
+server3.listen(0, common.mustCall(() => {
+  const session = http2.connect(`http://localhost:${server3.address().port}`);
+  const req = session.request();
+  req.on('error', common.mustCall((e) => {
+    assert.strictEqual(e.code, 'ERR_HTTP2_STREAM_ERROR');
+    server3.close();
+    session.close();
+  }));
+}));