Merge pull request #3646 from dellalibera/master

[javascript] CodeQL query to detect missing origin validation in cross-origin communication via postMessage

javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.qhelp

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+
+<p>If you use cross-origin communication between Window objects and do expect to receive messages from other sites, always verify the sender's identity using the origin and possibly source properties of the recevied `MessageEvent`. </p>
+
+<p>Unexpected behaviours, like `DOM-based XSS` could occur, if the event handler for incoming data does not check the origin of the data received and handles the data in an unsafe way.</p>
+</overview>
+
+<recommendation>
+<p>
+Always verify the sender's identity of incoming messages.
+</p>
+
+</recommendation>
+
+<example>
+<p>In the first example, the `MessageEvent.data` is passed to the `eval` function withouth checking the origin. This means that any window can send arbitrary messages that will be executed in the window receiving the message</p>
+<sample src="examples/postMessageNoOriginCheck.js" />
+
+<p> In the second example, the `MessageEvent.origin` is verified with an unsecure check. For example, using `event.origin.indexOf('www.example.com') > -1` can be bypassed because the string `www.example.com` could appear anywhere in `event.origin` (i.e. `www.example.com.mydomain.com`)</p>
+<sample src="examples/postMessageInsufficientCheck.js" />
+
+<p> In the third example, the `MessageEvent.origin` is properly checked against a trusted origin. </p>
+<sample src="examples/postMessageWithOriginCheck.js" />
+
+</example>
+
+<references>
+
+<li><a href="https://cwe.mitre.org/data/definitions/20.html">CWE-20: Improper Input Validation</a></li>
+<li><a href="https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage">Window.postMessage()</a></li>
+<li><a href="https://portswigger.net/web-security/dom-based/web-message-manipulation">Web-message manipulation</a></li>
+<li><a href="https://labs.detectify.com/2016/12/08/the-pitfalls-of-postmessage/">The pitfalls of postMessage</a></li>
+
+</references>
+</qhelp>

javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.ql

@@ -0,0 +1,64 @@
+/**
+ * @name Missing `MessageEvent.origin` verification in `postMessage` handlers
+ * @description Missing the `MessageEvent.origin` verification in `postMessage` handlers, allows any windows to send arbitrary data to the `MessageEvent` listener.
+ * This could lead to unexpected behaviour, especially when `MessageEvent.data` is used in an unsafe way.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/missing-postmessageorigin-verification
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-20
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.DOM
+
+/**
+ * A method call for the insecure functions used to verify the `MessageEvent.origin`.
+ */
+class InsufficientOriginChecks extends DataFlow::Node {
+  InsufficientOriginChecks() {
+    exists(DataFlow::Node node |
+      this.(StringOps::StartsWith).getSubstring() = node or
+      this.(StringOps::Includes).getSubstring() = node or
+      this.(StringOps::EndsWith).getSubstring() = node
+    )
+  }
+}
+
+/**
+ * A function handler for the `MessageEvent`.
+ */
+class PostMessageHandler extends DataFlow::FunctionNode {
+  PostMessageHandler() { this.getFunction() instanceof PostMessageEventHandler }
+}
+
+/**
+ * The `MessageEvent` parameter received by the handler
+ */
+class PostMessageEvent extends DataFlow::SourceNode {
+  PostMessageEvent() { exists(PostMessageHandler handler | this = handler.getParameter(0)) }
+
+  /**
+   * Holds if an access on `MessageEvent.origin` is in an `EqualityTest` and there is no call of an insufficient verification method on `MessageEvent.origin`
+   */
+  predicate hasOriginChecked() {
+    exists(EqualityTest test |
+      this.getAPropertyRead(["origin", "source"]).flowsToExpr(test.getAnOperand())
+    )
+  }
+
+  /**
+   * Holds if there is an insufficient method call (i.e indexOf) used to verify `MessageEvent.origin`
+   */
+  predicate hasOriginInsufficientlyChecked() {
+    exists(InsufficientOriginChecks insufficientChecks |
+      this.getAPropertyRead("origin").getAMethodCall*() = insufficientChecks
+    )
+  }
+}
+
+from PostMessageEvent event
+where not event.hasOriginChecked() or event.hasOriginInsufficientlyChecked()
+select event, "Missing or unsafe origin verification."

javascript/ql/src/experimental/Security/CWE-020/examples/postMessageInsufficientCheck.js

@@ -0,0 +1,14 @@
+function postMessageHandler(event) {
+    let origin = event.origin.toLowerCase();
+
+    let host = window.location.host;
+
+    // BAD
+    if (origin.indexOf(host) === -1)
+        return;
+
+
+    eval(event.data);
+}
+
+window.addEventListener('message', postMessageHandler, false);

javascript/ql/src/experimental/Security/CWE-020/examples/postMessageNoOriginCheck.js

@@ -0,0 +1,9 @@
+function postMessageHandler(event) {
+    let origin = event.origin.toLowerCase();
+
+    console.log(origin)
+    // BAD: the origin property is not checked
+    eval(event.data);
+}
+
+window.addEventListener('message', postMessageHandler, false);

javascript/ql/src/experimental/Security/CWE-020/examples/postMessageWithOriginCheck.js

@@ -0,0 +1,9 @@
+function postMessageHandler(event) {
+    console.log(event.origin)
+    // GOOD: the origin property is checked
+    if (event.origin === 'www.example.com') {
+        // do something
+    }
+}
+
+window.addEventListener('message', postMessageHandler, false);