Merge pull request github#12854 from natejohnson05/js-insecure-http-parser

JS - NodeJS CWE-444 InsecureHTTPParser

Author: erik-krogh

javascript/ql/src/experimental/Security/CWE-444/InsecureHttpParser.qhelp

@@ -0,0 +1,63 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+		<p>
+
+			Strict HTTP parsing may cause problems with interoperability with some 
+			non-conformant HTTP implementations. But disabling it is strongly discouraged,
+			as it opens the door to several threats including HTTP Request Smuggling.
+
+		</p>
+
+	</overview>
+
+	<recommendation>
+
+		<p>
+
+			Do not enable insecure http parser.
+
+		</p>
+
+	</recommendation>
+
+	<example>
+
+		<p>
+
+			The following example shows the instantiation of an http server. This 
+			server is vulnerable to HTTP Request Smuggling because the
+			<code>insecureHTTPParser</code> option of the server instantiation is 
+			set to <code>true</code>. As a consequence, malformed packets may attempt
+			to exploit any number of weaknesses including ranging from Web Cache Poisoning
+			Attacks to bypassing firewall protection mecahanisms.
+
+		</p>
+
+		<sample src="examples/InsecureHttpParser.js"/>
+
+		<p>
+
+			To make sure that packets are parsed correctly, the
+			<code>invalidHTTPParser</code> option should have its default value,
+			or be explicitly set to <code>false</code>.
+
+		</p>
+
+	</example>
+
+	<references>
+
+		<li>NodeJS: <a href="https://nodejs.org/en/blog/vulnerability/february-2020-security-releases">February 20 Security Release</a></li>
+
+		<li>Snyk: <a href="https://snyk.io/blog/node-js-release-fixes-a-critical-http-security-vulnerability/">NodeJS Critical HTTP Vulnerability</a></li>
+
+		<li>CWE-444: <a href="https://cwe.mitre.org/data/definitions/444.html">HTTP Request/Response Smuggling</a></li>
+
+	</references>
+
+</qhelp>

javascript/ql/src/experimental/Security/CWE-444/InsecureHttpParser.ql

@@ -0,0 +1,42 @@
+/**
+ * @name Insecure http parser
+ * @description Using an insecure http parser can lead to http smuggling attacks.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 9.0
+ * @precision high
+ * @id js/insecure-http-parser
+ * @tags security
+ *       external/cwe/cwe-444
+ */
+
+import javascript
+
+/** Gets options argument for a potential http or https connection */
+DataFlow::InvokeNode nodeInvocation() {
+  result instanceof ClientRequest
+  or
+  result instanceof Http::ServerDefinition
+}
+
+/** Gets an options object for an http or https connection. */
+DataFlow::ObjectLiteralNode nodeOptions() { result.flowsTo(nodeInvocation().getAnArgument()) }
+
+from DataFlow::PropWrite disable
+where
+  exists(DataFlow::SourceNode env |
+    env = NodeJSLib::process().getAPropertyRead("env") and
+    disable = env.getAPropertyWrite("NODE_OPTIONS") and
+    disable.getRhs().getStringValue().matches("%--insecure-http-parser%")
+  )
+  or
+  (
+    disable = nodeOptions().getAPropertyWrite("insecureHTTPParser")
+    or
+    // the same thing, but with API-nodes if they happen to be available
+    exists(API::Node nodeInvk | nodeInvk.getAnInvocation() = nodeInvocation() |
+      disable.getRhs() = nodeInvk.getAParameter().getMember("insecureHTTPParser").asSink()
+    )
+  ) and
+  disable.getRhs().(AnalyzedNode).getTheBooleanValue() = true
+select disable, "Allowing invalid HTTP headers is strongly discouraged."

javascript/ql/src/experimental/Security/CWE-444/examples/InsecureHttpParser.js

@@ -0,0 +1,7 @@
+const http = require('node:http');
+
+http.createServer({
+    insecureHTTPParser: true
+}, (req, res) => {
+    res.end('hello world\n');
+});