github · erik-krogh · May 9, 2022 · Apr 8, 2022 · Apr 8, 2022 · Apr 8, 2022
@@ -1225,19 +1225,25 @@ module TaintTracking {
    * An equality test on `e.origin` or `e.source` where `e` is a `postMessage` event object,
    * considered as a sanitizer for `e`.
    */
-  private class PostMessageEventSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
+  private class PostMessageEventSanitizer extends AdditionalSanitizerGuardNode {
     VarAccess event;
-    override EqualityTest astNode;
+    boolean polarity;
 
     PostMessageEventSanitizer() {
-      exists(string prop | prop = "origin" or prop = "source" |
-        astNode.getAnOperand().(PropAccess).accesses(event, prop) and
-        event.mayReferToParameter(any(PostMessageEventHandler h).getEventParameter())
+      event.mayReferToParameter(any(PostMessageEventHandler h).getEventParameter()) and
+      exists(DataFlow::PropRead read | read.accesses(event.flow(), ["origin", "source"]) |
+        exists(EqualityTest test | polarity = test.getPolarity() and this.getAstNode() = test |
+          test.getAnOperand().flow() = read
+        )
+        or
+        exists(InclusionTest test | polarity = test.getPolarity() and this = test |
+          test.getContainedNode() = read
+        )
       )
     }
 
     override predicate sanitizes(boolean outcome, Expr e) {
-      outcome = astNode.getPolarity() and
+      outcome = polarity and
       e = event
     }
 

@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+
+<p>
+The <code>"message"</code> event is used to send messages between windows. 
+An untrusted window can send a message to a trusted window, and it is up to the receiver to verify the legitimacy of the message. One way of performing that verification is to check the <code>origin</code> of the message ensure that it originates from a trusted window.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Always verify the origin of incoming messages.
+</p>
+</recommendation>
+
+<example>
+<p>
+The example below uses a received message to execute some code. However, the 
+origin of the message is not checked, so it might be possible for an attacker
+to execute arbitrary code.
+</p>
+<sample src="examples/MissingOriginCheckBad.js" />
+
+<p>
+The example is fixed below, where the origin is checked to be trusted. 
+It is therefore not possible for a malicious user to perform an attack using an untrusted origin.
+</p>
+<sample src="examples/MissingOriginCheckGood.js" />
+
+</example>
+
+<references>
+
+<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>
@@ -0,0 +1,83 @@
+/**
+ * @name Missing origin verification in `postMessage` handler
+ * @description Missing origin verification in a `postMessage` handler allows any windows to send arbitrary data to the handler.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 5
+ * @precision medium
+ * @id js/missing-origin-check
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-020
+ */
+
+import javascript
+
+/** A function that handles "message" events. */
+class PostMessageHandler extends DataFlow::FunctionNode {
+  override PostMessageEventHandler astNode;
+
+  /** Gets the parameter that contains the event. */
+  DataFlow::ParameterNode getEventParameter() {
+    result = DataFlow::parameterNode(astNode.getEventParameter())
+  }
+}
+
+/** Gets a reference to the event from a postmessage `handler` */
+DataFlow::SourceNode event(DataFlow::TypeTracker t, PostMessageHandler handler) {
+  t.start() and
+  result = handler.getEventParameter()
+  or
+  exists(DataFlow::TypeTracker t2 | result = event(t2, handler).track(t2, t))
+}
+
+/** Gets a reference to the .origin from a postmessage event. */
+DataFlow::SourceNode origin(DataFlow::TypeTracker t, PostMessageHandler handler) {
+  t.start() and
+  result = event(DataFlow::TypeTracker::end(), handler).getAPropertyRead("origin")
+  or
+  result =
+    origin(t.continue(), handler)
+        .getAMethodCall([
+            "toString", "toLowerCase", "toUpperCase", "toLocaleLowerCase", "toLocaleUpperCase"
+          ])
+  or
+  exists(DataFlow::TypeTracker t2 | result = origin(t2, handler).track(t2, t))
+}
+
+/** Gets a reference to the .source from a postmessage event. */
+DataFlow::SourceNode source(DataFlow::TypeTracker t, PostMessageHandler handler) {
+  t.start() and
+  result = event(DataFlow::TypeTracker::end(), handler).getAPropertyRead("source")
+  or
+  exists(DataFlow::TypeTracker t2 | result = source(t2, handler).track(t2, t))
+}
+
+/** Gets a reference to the origin or the source of a postmessage event. */
+DataFlow::SourceNode sourceOrOrigin(PostMessageHandler handler) {
+  result = source(DataFlow::TypeTracker::end(), handler) or
+  result = origin(DataFlow::TypeTracker::end(), handler)
+}
+
+/** Holds if there exists a check of the .origin or .source of the postmessage `handler`. */
+predicate hasOriginCheck(PostMessageHandler handler) {
+  // event.origin === "constant"
+  exists(EqualityTest test | sourceOrOrigin(handler).flowsToExpr(test.getAnOperand()))
+  or
+  // set.includes(event.source)
+  exists(InclusionTest test | sourceOrOrigin(handler).flowsTo(test.getContainedNode()))
+  or
+  // "safeOrigin".startsWith(event.origin)
+  exists(StringOps::StartsWith starts |
+    origin(DataFlow::TypeTracker::end(), handler).flowsTo(starts.getSubstring())
+  )
+  or
+  // "safeOrigin".endsWith(event.origin)
+  exists(StringOps::EndsWith ends |
+    origin(DataFlow::TypeTracker::end(), handler).flowsTo(ends.getSubstring())
+  )
+}
+
+from PostMessageHandler handler
+where not hasOriginCheck(handler)
+select handler.getEventParameter(), "Postmessage handler has no origin check."
@@ -1,7 +1,7 @@
 function postMessageHandler(event) {
     console.log(event.origin)
     // GOOD: the origin property is checked
-    if (event.origin === 'www.example.com') {
+    if (event.origin === 'https://www.example.com') {
         // do something
     }
 }

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* The `js/missing-origin-check` query has been added. It highlights "message" event handlers that do not check the origin of the event.  
+  The query previously existed as the experimental `js/missing-postmessageorigin-verification` query.
@@ -0,0 +1,3 @@
+| tst.js:11:20:11:24 | event | Postmessage handler has no origin check. |
+| tst.js:24:27:24:27 | e | Postmessage handler has no origin check. |
+| tst.js:40:27:40:27 | e | Postmessage handler has no origin check. |
@@ -0,0 +1 @@
+Security/CWE-020/MissingOriginCheck.ql
@@ -0,0 +1,70 @@
+window.onmessage = event => { // OK - good origin check
+    let origin = event.origin.toLowerCase();
+
+    if (origin !== window.location.origin) {
+        return;
+    }
+
+    eval(event.data);
+}
+
+window.onmessage = event => { // NOT OK - no origin check
+    let origin = event.origin.toLowerCase();
+
+    console.log(origin);
+    eval(event.data);
+}
+
+window.onmessage = event => { // OK - there is an origin check
+    if (event.origin === "https://www.example.com") {
+      // do something
+    }
+}
+
+self.onmessage = function(e) { // NOT OK
+    Commands[e.data.cmd].apply(null, e.data.args);
+};
+
+window.onmessage = event => { // OK - there is an origin check
+    if (mySet.includes(event.origin)) {
+        // do something
+    }
+}
+
+window.onmessage = event => { // OK - there is an origin check
+    if (mySet.includes(event.source)) {
+        // do something
+    }
+}
+
+self.onmessage = function(e) { // NOT OK
+    Commands[e.data.cmd].apply(null, e.data.args);
+};
+
+window.addEventListener('message', function(e) { // OK - has a good origin check
+    if (is_sysend_post_message(e) && is_valid_origin(e.origin)) {
+        var payload = JSON.parse(e.data);
+        if (payload && payload.name === uniq_prefix) {
+            var data = unserialize(payload.data);
+            sysend.broadcast(payload.key, data);
+        }
+    }
+});
+
+function is_valid_origin(origin) {
+    if (!domains) {
+        warn("no domains configured");
+        return true;
+    }
+    var valid = domains.includes(origin);
+    if (!valid) {
+        warn("invalid origin: " + origin);
+    }
+    return valid;
+}
+
+window.onmessage = event => { // OK - the check is OK
+    if ("https://www.example.com".startsWith(event.origin)) {
+      // do something
+    }
+}
@@ -1,13 +1,3 @@
-| tst-IncompleteHostnameRegExp.js:2:2:2:24 | /^http: ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:12:14:12:39 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:15:22:15:47 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:19:17:19:35 | '^test.example.com' | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:40:2:40:30 | /^https ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
-| tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
 | tst-SemiAnchoredRegExp.js:3:2:3:7 | /^a\|b/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not |
 | tst-SemiAnchoredRegExp.js:6:2:6:9 | /^a\|b\|c/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not |
 | tst-SemiAnchoredRegExp.js:12:2:12:9 | /^a\|(b)/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not |

@@ -1,6 +1,3 @@
-| tst-IncompleteHostnameRegExp.js:42:13:42:65 | '^http[ ... \\/(.+)' | The escape sequence '\\/' is equivalent to just '/'. |
-| tst-SemiAnchoredRegExp.js:72:13:72:40 | '^good\\ ... \\\\.com' | The escape sequence '\\.' is equivalent to just '.'. |
-| tst-SemiAnchoredRegExp.js:109:2:109:45 | /^((\\+\| ... ?\\d\\d)/ | The escape sequence '\\:' is equivalent to just ':'. |
 | tst-escapes.js:19:8:19:11 | "\\ " | The escape sequence '\\ ' is equivalent to just ' '. |
 | tst-escapes.js:20:1:20:54 | /\\a\\b\\c ... x\\y\\z"/ | The escape sequence '\\a' is equivalent to just 'a'. |
 | tst-escapes.js:20:1:20:54 | /\\a\\b\\c ... x\\y\\z"/ | The escape sequence '\\e' is equivalent to just 'e'. |