Merge pull request github#8773 from erik-krogh/exhaustion

JS: promote `js/resource-exhaustion` out of experimental

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll

@@ -0,0 +1,127 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * resource exhaustion vulnerabilities, as well as extension points for
+ * adding your own.
+ */
+
+import javascript
+
+/**
+ * Provides sources, sinks, and sanitizers for reasoning about
+ * resource exhaustion vulnerabilities.
+ */
+module ResourceExhaustion {
+  /**
+   * A data flow source for resource exhaustion vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for resource exhaustion vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node {
+    /**
+     * Gets a description of why this is a problematic sink.
+     */
+    abstract string getProblemDescription();
+  }
+
+  /**
+   * A data flow sanitizer for resource exhaustion vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /** A source of remote user input, considered as a data flow source for resource exhaustion vulnerabilities. */
+  class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource {
+    RemoteFlowSourceAsSource() {
+      // exclude source that only happen client-side
+      not this instanceof ClientSideRemoteFlowSource and
+      not this = DataFlow::parameterNode(any(PostMessageEventHandler pmeh).getEventParameter())
+    }
+  }
+
+  /**
+   * A node that determines the repetitions of a string, considered as a data flow sink for resource exhaustion vulnerabilities.
+   */
+  class StringRepetitionSink extends Sink {
+    StringRepetitionSink() {
+      exists(DataFlow::MethodCallNode repeat |
+        repeat.getMethodName() = "repeat" and
+        this = repeat.getArgument(0)
+      )
+    }
+
+    override string getProblemDescription() {
+      result = "This creates a string with a user-controlled length"
+    }
+  }
+
+  /**
+   * A node that determines the duration of a timer, considered as a data flow sink for resource exhaustion vulnerabilities.
+   */
+  class TimerDurationSink extends Sink {
+    TimerDurationSink() {
+      this = DataFlow::globalVarRef(["setTimeout", "setInterval"]).getACall().getArgument(1) or
+      this = LodashUnderscore::member(["delay", "throttle", "debounce"]).getACall().getArgument(1)
+    }
+
+    override string getProblemDescription() {
+      result = "This creates a timer with a user-controlled duration"
+    }
+  }
+
+  /**
+   * A node that determines the size of a buffer, considered as a data flow sink for resource exhaustion vulnerabilities.
+   */
+  class BufferSizeSink extends Sink {
+    BufferSizeSink() {
+      exists(DataFlow::SourceNode clazz, DataFlow::InvokeNode invk, int index |
+        clazz = DataFlow::globalVarRef("Buffer") and this = invk.getArgument(index)
+      |
+        exists(string name |
+          invk = clazz.getAMemberCall(name) and
+          (
+            name = "from" and index = 2 // the length argument
+            or
+            name = ["alloc", "allocUnsafe", "allocUnsafeSlow"] and index = 0 // the buffer size
+          )
+        )
+        or
+        invk = clazz.getAnInvocation() and
+        (
+          // invk.getNumArgument() = 1 and // `new Buffer(size)`, it's only an issue if the size is a number, which we don't track precisely.
+          // index = 0
+          // or
+          invk.getNumArgument() = 3 and index = 2 // the length argument
+        )
+      )
+      or
+      this = DataFlow::globalVarRef("SlowBuffer").getAnInstantiation().getArgument(0)
+    }
+
+    override string getProblemDescription() {
+      result = "This creates a buffer with a user-controlled size"
+    }
+  }
+
+  /**
+   * A node that determines the size of an array, considered as a data flow sink for resource exhaustion vulnerabilities.
+   * This is only an issue if the argument is a number, which we don't track precisely.
+   */
+  class DenseArraySizeSink extends Sink {
+    DenseArraySizeSink() {
+      // Arrays are sparse by default, so we must also look at how the array is used
+      exists(DataFlow::ArrayConstructorInvokeNode instance |
+        this = instance.getArgument(0) and
+        instance.getNumArgument() = 1
+      |
+        exists(instance.getAMethodCall(["map", "fill", "join", "toString"])) or
+        instance.flowsToExpr(any(AddExpr p).getAnOperand())
+      )
+    }
+
+    override string getProblemDescription() {
+      result = "This creates an array with a user-controlled length"
+    }
+  }
+}

javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionQuery.qll

@@ -0,0 +1,65 @@
+/**
+ * Provides a taint tracking configuration for reasoning about
+ * resource exhaustion vulnerabilities (CWE-770).
+ *
+ * Note, for performance reasons: only import this file if
+ * `ResourceExhaustion::Configuration` is needed, otherwise
+ * `ResourceExhaustionCustomizations` should be imported instead.
+ */
+
+import javascript
+import ResourceExhaustionCustomizations::ResourceExhaustion
+
+/**
+ * A data flow configuration for resource exhaustion vulnerabilities.
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "ResourceExhaustion" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof Sanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) {
+    isNumericFlowStep(src, dst)
+  }
+
+  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
+    guard instanceof UpperBoundsCheckSanitizerGuard
+  }
+
+  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
+    succ.(DataFlow::PropRead).accesses(pred, "length")
+  }
+}
+
+/** Holds if data is converted to a number from `src` to `dst`. */
+predicate isNumericFlowStep(DataFlow::Node src, DataFlow::Node dst) {
+  exists(DataFlow::CallNode c |
+    c = dst and
+    src = c.getAnArgument()
+  |
+    c = DataFlow::globalVarRef("Math").getAMemberCall(_) or
+    c = DataFlow::globalVarRef(["Number", "parseInt", "parseFloat"]).getACall()
+  )
+}
+
+/**
+ * A sanitizer that blocks taint flow if the size of a number is limited.
+ */
+class UpperBoundsCheckSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
+  override RelationalComparison astNode;
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    true = outcome and
+    e = astNode.getLesserOperand()
+    or
+    false = outcome and
+    e = astNode.getGreaterOperand()
+  }
+}

javascript/ql/src/Security/CWE-770/ResourceExhaustion.qhelp

@@ -0,0 +1,115 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+		<p>
+
+			Applications are constrained by how many resources they can make use
+			of. Failing to respect these constraints may cause the application to
+			be unresponsive or crash. It is therefore problematic if attackers
+			can control the sizes or lifetimes of allocated objects.
+
+		</p>
+
+	</overview>
+
+	<recommendation>
+
+		<p>
+
+			Ensure that attackers can not control object sizes and their
+			lifetimes. If object sizes and lifetimes must be controlled by
+			external parties, ensure you restrict the object sizes and lifetimes so that
+			they are within acceptable ranges.
+
+		</p>
+
+	</recommendation>
+
+	<example>
+
+		<p>
+
+			The following example allocates a buffer with a user-controlled
+			size.
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_buffer.js" />
+
+		<p>
+
+			This is problematic since an attacker can choose a size
+			that makes the application run out of memory. Even worse, in older
+			versions of Node.js, this could leak confidential memory.
+
+			To prevent such attacks, limit the buffer size:
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_buffer_fixed.js" />
+
+	</example>
+
+	<example>
+
+		<p>
+
+			As another example, consider an application that allocates an
+			array with a user-controlled size, and then fills it with values:
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_array.js" />
+
+		<p>
+			The allocation of the array itself is not problematic since arrays are
+			allocated sparsely, but the subsequent filling of the array will take
+			a long time, causing the application to be unresponsive, or even run
+			out of memory.
+
+			Again, a limit on the size will prevent the attack:
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_array_fixed.js" />
+
+	</example>
+
+	<example>
+
+		<p>
+
+			Finally, the following example lets a user choose a delay after
+            which a function is executed:
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_timeout.js" />
+
+		<p>
+
+			This is problematic because a large delay essentially makes the
+			application wait indefinitely before executing the function. Repeated
+			registrations of such delays will therefore use up all of the memory
+			in the application.
+
+			A limit on the delay will prevent the attack:
+
+		</p>
+
+		<sample src="examples/ResourceExhaustion_timeout_fixed.js" />
+
+
+	</example>
+
+	<references>
+        <li>
+            Wikipedia: <a href="https://en.wikipedia.org/wiki/Denial-of-service_attack">Denial-of-service attack</a>.
+        </li>
+	</references>
+
+</qhelp>

javascript/ql/src/experimental/Security/CWE-770/ResourceExhaustion.ql renamed to javascript/ql/src/Security/CWE-770/ResourceExhaustion.ql

@@ -4,15 +4,17 @@
  *              sizes or durations can cause resource exhaustion.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 7.5
  * @id js/resource-exhaustion
  * @precision high
  * @tags security
+ *       external/cwe/cwe-400
  *       external/cwe/cwe-770
  */
 
 import javascript
 import DataFlow::PathGraph
-import experimental.semmle.javascript.security.dataflow.ResourceExhaustion::ResourceExhaustion
+import semmle.javascript.security.dataflow.ResourceExhaustionQuery
 
 from Configuration dataflow, DataFlow::PathNode source, DataFlow::PathNode sink
 where dataflow.hasFlowPath(source, sink)

javascript/ql/src/Security/CWE-770/examples/ResourceExhaustion_array.js

@@ -0,0 +1,10 @@
+var http = require("http"),
+    url = require("url");
+
+var server = http.createServer(function(req, res) {
+	var size = parseInt(url.parse(req.url, true).query.size);
+
+	let dogs = new Array(size).fill("dog"); // BAD
+
+	// ... use the dog
+});

javascript/ql/src/Security/CWE-770/examples/ResourceExhaustion_array_fixed.js

@@ -0,0 +1,16 @@
+var http = require("http"),
+    url = require("url");
+
+var server = http.createServer(function(req, res) {
+	var size = parseInt(url.parse(req.url, true).query.size);
+
+	if (size > 1024) {
+		res.statusCode = 400;
+		res.end("Bad request.");
+		return;
+	}
+
+	let dogs = new Array(size).fill("dog"); // GOOD
+
+	// ... use the dogs
+});

javascript/ql/src/Security/CWE-770/examples/ResourceExhaustion_buffer.js

@@ -0,0 +1,10 @@
+var http = require("http"),
+    url = require("url");
+
+var server = http.createServer(function(req, res) {
+	var size = parseInt(url.parse(req.url, true).query.size);
+
+	let buffer = Buffer.alloc(size); // BAD
+
+	// ... use the buffer
+});

javascript/ql/src/Security/CWE-770/examples/ResourceExhaustion_buffer_fixed.js

@@ -0,0 +1,16 @@
+var http = require("http"),
+    url = require("url");
+
+var server = http.createServer(function(req, res) {
+	var size = parseInt(url.parse(req.url, true).query.size);
+
+	if (size > 1024) {
+		res.statusCode = 400;
+		res.end("Bad request.");
+		return;
+	}
+
+	let buffer = Buffer.alloc(size); // GOOD
+
+	// ... use the buffer
+});