Add Codeql to detect missing 'Message.origin' validation when using postMessage API

Author: dellalibera

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

@@ -0,0 +1,37 @@
+<!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 checked against a trusted origin. 
+<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,70 @@
+/**
+ * @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
+
+/**
+ * A call to a `window` event listener
+ */
+class WindowListeners extends DataFlow::CallNode {
+  WindowListeners() {
+    exists(DataFlow::SourceNode source |
+      source = DataFlow::globalVarRef("window") and
+      this = source.getAMethodCall("addEventListener")
+    )
+  }
+}
+
+/**
+ * A call to a `window` event listener for the `MessageEvent`
+ */
+class PostMessageListeners extends DataFlow::CallNode {
+  PostMessageListeners() {
+    exists(WindowListeners listener |
+      listener.getArgument(0).mayHaveStringValue("message") and
+      this = listener
+    )
+  }
+}
+
+/**
+ * A function handler for the `MessageEvent`. It is the second argument of a `MessageEvent` listener
+ */
+class PostMessageHandler extends DataFlow::FunctionNode {
+  PostMessageHandler() {
+    exists(PostMessageListeners listener | this = listener.getArgument(1).getAFunctionValue())
+  }
+}
+
+/**
+ * The `MessageEvent` received by the handler
+ */
+class PostMessageEvent extends DataFlow::SourceNode {
+  PostMessageEvent() { exists(PostMessageHandler handler | this = handler.getParameter(0)) }
+
+  /**
+   * Holds if a call to a method on `MessageEvent.origin` or a read of `MessageEvent.origin` is in an `if` statement
+   */
+  predicate isOriginChecked() {
+    exists(IfStmt ifStmt |
+      ifStmt = this.getAPropertyRead("origin").getAMethodCall*().asExpr().getEnclosingStmt() or
+      ifStmt = this.getAPropertyRead("origin").asExpr().getEnclosingStmt()
+    )
+  }
+}
+
+from PostMessageEvent event, PostMessageHandler handler
+where
+  event = handler.getParameter(0) and
+  not event.isOriginChecked()
+select "The `MessageEvent.origin` property is not checked.", event, handler

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);