diff --git a/change-notes/1.20/analysis-javascript.md b/change-notes/1.20/analysis-javascript.md index 30874ce9c51..035c500e6c4 100644 --- a/change-notes/1.20/analysis-javascript.md +++ b/change-notes/1.20/analysis-javascript.md @@ -14,6 +14,7 @@ | **Query** | **Tags** | **Purpose** | |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Cross-window communication with unrestricted target origin (`js/cross-window-information-leak`) | security, external/cwe/359 | Highlights code that sends potentially sensitive information to another window without restricting the receiver window's origin. Results are shown on LGTM by default. | | Double escaping or unescaping (`js/double-escaping`) | correctness, security, external/cwe/cwe-116 | Highlights potential double escaping or unescaping of special characters, indicating a possible violation of [CWE-116](https://cwe.mitre.org/data/definitions/116.html). Results are shown on LGTM by default. | | Incomplete regular expression for hostnames (`js/incomplete-hostname-regexp`) | correctness, security, external/cwe/cwe-020 | Highlights hostname sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default.| | Incomplete URL substring sanitization | correctness, security, external/cwe/cwe-020 | Highlights URL sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default. | diff --git a/javascript/config/suites/javascript/security b/javascript/config/suites/javascript/security index 2e010b5abd8..77c352a61a8 100644 --- a/javascript/config/suites/javascript/security +++ b/javascript/config/suites/javascript/security @@ -22,6 +22,7 @@ + semmlecode-javascript-queries/Security/CWE-338/InsecureRandomness.ql: /Security/CWE/CWE-338 + semmlecode-javascript-queries/Security/CWE-346/CorsMisconfigurationForCredentials.ql: /Security/CWE/CWE-346 + semmlecode-javascript-queries/Security/CWE-352/MissingCsrfMiddleware.ql: /Security/CWE/CWE-352 ++ semmlecode-javascript-queries/Security/CWE-359/PostMessageStar.ql: /Security/CWE/CWE-359 + semmlecode-javascript-queries/Security/CWE-400/RemotePropertyInjection.ql: /Security/CWE/CWE-400 + semmlecode-javascript-queries/Security/CWE-502/UnsafeDeserialization.ql: /Security/CWE/CWE-502 + semmlecode-javascript-queries/Security/CWE-506/HardcodedDataInterpretedAsCode.ql: /Security/CWE/CWE-506 diff --git a/javascript/ql/src/Security/CWE-359/PostMessageStar.qhelp b/javascript/ql/src/Security/CWE-359/PostMessageStar.qhelp new file mode 100644 index 00000000000..c3a0bcb0268 --- /dev/null +++ b/javascript/ql/src/Security/CWE-359/PostMessageStar.qhelp @@ -0,0 +1,56 @@ + + + + +

+The window.postMessage method allows different windows or iframes +to communicate directly, even if they were loaded from different origins, circumventing +the usual same-origin policy. +

+

+The sender of the message can restrict the origin of the receiver by specifying +a target origin. If the receiver window does not come from this origin, the +message is not sent. +

+

+Alternatively, the sender can specify a target origin of '*', which means +that any origin is acceptable and the message is always sent. +

+

+This feature should not be used if the message being sent contains sensitive data such +as user credentials: the target window may have been loaded from a malicious site, +to which the data would then become available. +

+ + + +

+If possible, specify a target origin when using window.postMessage. +Alternatively, encrypt the sensitive data before sending it to prevent an unauthorized +receiver from accessing it. +

+ + + +

+The following example code sends user credentials (in this case, their user +name) to window.parent without checking its origin. If a malicious site +loads the page containing this code into an iframe it would be able to gain access +to the user name. +

+ +

+To prevent this from happening, the origin of the target window should be restricted, +as in this example: +

+ + + + + +
  • Mozilla Developer Network: Window.postMessage.
    • +
  • Mozilla Developer Network: Same-origin policy.
    • +     +     diff --git a/javascript/ql/src/Security/CWE-359/PostMessageStar.ql b/javascript/ql/src/Security/CWE-359/PostMessageStar.ql new file mode 100644 index 00000000000..6705079cff9 --- /dev/null +++ b/javascript/ql/src/Security/CWE-359/PostMessageStar.ql @@ -0,0 +1,22 @@ +/** + * @name Cross-window communication with unrestricted target origin + * @description When sending sensitive information to another window using `postMessage`, + * the origin of the target window should be restricted to avoid unintentional + * information leaks. + * @kind path-problem + * @problem.severity error + * @precision high + * @id js/cross-window-information-leak + * @tags security + * external/cwe/cwe-359 + */ + +import javascript +import semmle.javascript.security.dataflow.PostMessageStar::PostMessageStar +import DataFlow::PathGraph + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink.getNode(), source, sink, + "Sensitive data returned from $@ is sent to another window without origin restriction.", + source.getNode(), "here" diff --git a/javascript/ql/src/Security/CWE-359/examples/PostMessageStar.js b/javascript/ql/src/Security/CWE-359/examples/PostMessageStar.js new file mode 100644 index 00000000000..63aa2666c69 --- /dev/null +++ b/javascript/ql/src/Security/CWE-359/examples/PostMessageStar.js @@ -0,0 +1 @@ +window.parent.postMessage(userName, '*'); diff --git a/javascript/ql/src/Security/CWE-359/examples/PostMessageStarGood.js b/javascript/ql/src/Security/CWE-359/examples/PostMessageStarGood.js new file mode 100644 index 00000000000..a904f896c27 --- /dev/null +++ b/javascript/ql/src/Security/CWE-359/examples/PostMessageStarGood.js @@ -0,0 +1 @@ +window.parent.postMessage(userName, 'https://lgtm.com'); diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/PostMessageStar.qll b/javascript/ql/src/semmle/javascript/security/dataflow/PostMessageStar.qll new file mode 100644 index 00000000000..c273ccb56e1 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/PostMessageStar.qll @@ -0,0 +1,84 @@ +/** + * Provides a taint tracking configuration for reasoning about cross-window communication + * with unrestricted origin. + */ + +import javascript +private import semmle.javascript.security.SensitiveActions + +module PostMessageStar { + /** + * A data flow source for cross-window communication with unrestricted origin. + */ + abstract class Source extends DataFlow::Node { } + + /** + * A data flow sink for cross-window communication with unrestricted origin. + */ + abstract class Sink extends DataFlow::Node { } + + /** + * A sanitizer for cross-window communication with unrestricted origin. + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** + * A taint tracking configuration for cross-window communication with unrestricted origin. + * + * This configuration identifies flows from `Source`s, which are sources of + * sensitive data, to `Sink`s, which is an abstract class representing all + * the places sensitive data may be transmitted across window boundaries without restricting + * the origin. + * + * Additional sources or sinks can be added either by extending the relevant class, or by subclassing + * this configuration itself, and amending the sources and sinks. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "PostMessageStar" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } + } + + /** + * A sensitive expression, viewed as a data flow source for cross-window communication + * with unrestricted origin. + */ + class SensitiveExprSource extends Source, DataFlow::ValueNode { override SensitiveExpr astNode; } + + /** + * A variable/property access or function call whose name suggests that it may contain credentials, + * viewed as a data flow source for cross-window communication with unrestricted origin. + */ + class CredentialsSource extends Source { + CredentialsSource() { + exists(string name | + name = this.(DataFlow::InvokeNode).getCalleeName() or + name = this.(DataFlow::PropRead).getPropertyName() or + name = this.asExpr().(VarUse).getVariable().getName() + | + name.regexpMatch(HeuristicNames::suspiciousCredentials()) and + not name.regexpMatch(HeuristicNames::nonSuspicious()) + ) + } + } + + /** A call to any function whose name suggests that it encodes or encrypts its arguments. */ + class ProtectSanitizer extends Sanitizer { ProtectSanitizer() { this instanceof ProtectCall } } + + /** + * An expression sent using `postMessage` without restricting the target window origin. + */ + class PostMessageStarSink extends Sink { + PostMessageStarSink() { + exists(DataFlow::MethodCallNode postMessage | + postMessage.getMethodName() = "postMessage" and + postMessage.getArgument(1).mayHaveStringValue("*") and + this = postMessage.getArgument(0) + ) + } + } +} diff --git a/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.expected b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.expected new file mode 100644 index 00000000000..4a6376cec72 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.expected @@ -0,0 +1,7 @@ +nodes +| PostMessageStar2.js:1:27:1:34 | password | +| PostMessageStar.js:1:27:1:34 | userName | +edges +#select +| PostMessageStar2.js:1:27:1:34 | password | PostMessageStar2.js:1:27:1:34 | password | PostMessageStar2.js:1:27:1:34 | password | Sensitive data returned from $@ is sent to another window without origin restriction. | PostMessageStar2.js:1:27:1:34 | password | here | +| PostMessageStar.js:1:27:1:34 | userName | PostMessageStar.js:1:27:1:34 | userName | PostMessageStar.js:1:27:1:34 | userName | Sensitive data returned from $@ is sent to another window without origin restriction. | PostMessageStar.js:1:27:1:34 | userName | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.js b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.js new file mode 100644 index 00000000000..63aa2666c69 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.js @@ -0,0 +1 @@ +window.parent.postMessage(userName, '*'); diff --git a/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.qlref b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.qlref new file mode 100644 index 00000000000..831b437cdfb --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar.qlref @@ -0,0 +1 @@ +Security/CWE-359/PostMessageStar.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar2.js b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar2.js new file mode 100644 index 00000000000..edb66c6a613 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStar2.js @@ -0,0 +1 @@ +window.parent.postMessage(password, '*'); diff --git a/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStarGood.js b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStarGood.js new file mode 100644 index 00000000000..a904f896c27 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-359/PostMessageStarGood.js @@ -0,0 +1 @@ +window.parent.postMessage(userName, 'https://lgtm.com');