Merge branch 'main' into stdlib-optparse

python/ql/src/CHANGELOG.md

@@ -1,3 +1,44 @@
+## 1.2.2
+
+### Minor Analysis Improvements
+
+* The `py/clear-text-logging-sensitive-data` and `py/clear-text-storage-sensitive-data` queries have been updated to exclude the `certificate` classification of sensitive sources, which often do not contain sensitive data.
+
+## 1.2.1
+
+No user-facing changes.
+
+## 1.2.0
+
+### New Queries
+
+* The `py/cookie-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack. This query finds instances of cookies being set without the `Secure`, `HttpOnly`, or `SameSite` attributes set to secure values.
+
+## 1.1.0
+
+### New Queries
+
+* The `py/cookie-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack. This query finds instances of cookies being constructed from user input. 
+
+### Minor Analysis Improvements
+
+* Added models of `streamlit` PyPI package.
+
+## 1.0.4
+
+No user-facing changes.
+
+## 1.0.3
+
+### Minor Analysis Improvements
+
+* Adding Python support for Hardcoded Credentials as Models as Data
+* Additional sanitizers have been added to the `py/full-ssrf` and `py/partial-ssrf` queries for methods that verify a string contains only a certain set of characters, such as `.isalnum()` as well as regular expression tests.
+
+## 1.0.2
+
+No user-facing changes.
+
 ## 1.0.1
 
 ### Minor Analysis Improvements

python/ql/src/Security/CWE-020/CookieInjection.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Constructing cookies from user input can allow an attacker to control a user's cookie. 
+This may lead to a session fixation attack. Additionally, client code may not expect a cookie to contain attacker-controlled data, and fail to sanitize it for common vulnerabilities such as Cross Site Scripting (XSS).
+An attacker manipulating the raw cookie header may additionally be able to set cookie attributes such as <code>HttpOnly</code> to insecure values.  
+</p>
+</overview>
+
+<recommendation>
+<p>Do not use raw user input to construct cookies.</p>
+</recommendation>
+
+<example>
+<p>In the following cases, a cookie is constructed for a Flask response using user input. The first uses <code>set_cookie</code>,
+and the second sets a cookie's raw value through the <code>set-cookie</code> header.</p>
+<sample src="examples/CookieInjection.py" />
+</example>
+
+<references>
+<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Session_fixation">Session Fixation</a>.</li>
+</references>
+
+</qhelp>

python/ql/src/Security/CWE-020/CookieInjection.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Construction of a cookie using user-supplied input
+ * @description Constructing cookies from user input may allow an attacker to perform a Cookie Poisoning attack.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision high
+ * @security-severity 5.0
+ * @id py/cookie-injection
+ * @tags security
+ *       external/cwe/cwe-20
+ */
+
+import python
+import semmle.python.security.dataflow.CookieInjectionQuery
+import CookieInjectionFlow::PathGraph
+
+from CookieInjectionFlow::PathNode source, CookieInjectionFlow::PathNode sink
+where CookieInjectionFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "Cookie is constructed from a $@.", source.getNode(),
+  "user-supplied input"

python/ql/src/Security/CWE-020/examples/CookieInjection.py

@@ -2,15 +2,15 @@ from flask import request, make_response
 
 
 @app.route("/1")
-def true():
+def set_cookie():
     resp = make_response()
-    resp.set_cookie(request.args["name"],
+    resp.set_cookie(request.args["name"], # BAD: User input is used to set the cookie's name and value
                     value=request.args["name"])
     return resp
 
 
 @app.route("/2")
-def flask_make_response():
-    resp = make_response("hello")
-    resp.headers['Set-Cookie'] = f"{request.args['name']}={request.args['name']};"
+def set_cookie_header():
+    resp = make_response()
+    resp.headers['Set-Cookie'] = f"{request.args['name']}={request.args['name']};" # BAD: User input is used to set the raw cookie header.
     return resp

python/ql/src/Security/CWE-614/InsecureCookie.qhelp

@@ -4,10 +4,9 @@
 <qhelp>
 
 <overview>
-<p>Setting the 'secure' flag on a cookie to <code>False</code> can cause it to be sent in cleartext.
-Setting the 'httponly' flag on a cookie to <code>False</code> may allow attackers access it via JavaScript.
-Setting the 'samesite' flag on a cookie to <code>'None'</code> will make the cookie to be sent in third-party 
-contexts which may be attacker-controlled.</p>
+<p>Cookies without the <code>Secure</code> flag set may be transmittd using HTTP instead of HTTPS, which leaves it vulnerable to being read by a third party.</p>
+<p>Cookies without the <code>HttpOnly</code> flag set are accessible to JavaScript running in the same origin. In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script.</p>
+<p>Cookies with the <code>SameSite</code> attribute set to <code>'None'</code> will be sent with cross-origin requests, which can be controlled by third-party JavaScript code and allow for Cross-Site Request Forgery (CSRF) attacks.</p>
 </overview>
 
 <recommendation>
@@ -18,9 +17,8 @@ contexts which may be attacker-controlled.</p>
 </recommendation>
 
 <example>
-<p>This example shows two ways of adding a cookie to a Flask response. The first way uses <code>set_cookie</code>'s
-secure flag and the second adds the secure flag in the cookie's raw value.</p>
-<sample src="InsecureCookie.py" />
+<p>In the following examples, the cases marked GOOD show secure cookie attributes being set; whereas in the cases marked BAD they are not set.</p>
+<sample src="examples/InsecureCookie.py" />
 </example>
 
 <references>

python/ql/src/Security/CWE-614/InsecureCookie.ql

@@ -0,0 +1,51 @@
+/**
+ * @name Failure to use secure cookies
+ * @description Insecure cookies may be sent in cleartext, which makes them vulnerable to
+ *              interception.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 5.0
+ * @precision high
+ * @id py/insecure-cookie
+ * @tags security
+ *       external/cwe/cwe-614
+ *       external/cwe/cwe-1004
+ *       external/cwe/cwe-1275
+ */
+
+import python
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.Concepts
+
+predicate hasProblem(Http::Server::CookieWrite cookie, string alert, int idx) {
+  cookie.hasSecureFlag(false) and
+  alert = "Secure" and
+  idx = 0
+  or
+  cookie.hasHttpOnlyFlag(false) and
+  alert = "HttpOnly" and
+  idx = 1
+  or
+  cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and
+  alert = "SameSite" and
+  idx = 2
+}
+
+predicate hasAlert(Http::Server::CookieWrite cookie, string alert) {
+  exists(int numProblems | numProblems = strictcount(string p | hasProblem(cookie, p, _)) |
+    numProblems = 1 and
+    alert = any(string prob | hasProblem(cookie, prob, _)) + " attribute"
+    or
+    numProblems = 2 and
+    alert =
+      strictconcat(string prob, int idx | hasProblem(cookie, prob, idx) | prob, " and " order by idx)
+        + " attributes"
+    or
+    numProblems = 3 and
+    alert = "Secure, HttpOnly, and SameSite attributes"
+  )
+}
+
+from Http::Server::CookieWrite cookie, string alert
+where hasAlert(cookie, alert)
+select cookie, "Cookie is added without the " + alert + " properly set."

python/ql/src/Security/CWE-614/examples/InsecureCookie.py

@@ -0,0 +1,20 @@
+from flask import Flask, request, make_response, Response
+
+
+@app.route("/good1")
+def good1():
+    resp = make_response()
+    resp.set_cookie("name", value="value", secure=True, httponly=True, samesite='Strict') # GOOD: Attributes are securely set
+    return resp
+
+
+@app.route("/good2")
+def good2():
+    resp = make_response()
+    resp.headers['Set-Cookie'] = "name=value; Secure; HttpOnly; SameSite=Strict" # GOOD: Attributes are securely set 
+    return resp
+
+@app.route("/bad1")
+    resp = make_response()
+    resp.set_cookie("name", value="value", samesite='None') # BAD: the SameSite attribute is set to 'None' and the 'Secure' and 'HttpOnly' attributes are set to False by default.
+    return resp

python/ql/src/Security/CWE-798/HardcodedCredentials.ql

@@ -18,6 +18,7 @@ import semmle.python.dataflow.new.TaintTracking
 import semmle.python.filters.Tests
 private import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch
 private import semmle.python.dataflow.new.internal.Builtins::Builtins as Builtins
+private import semmle.python.frameworks.data.ModelsAsData
 
 bindingset[char, fraction]
 predicate fewer_characters_than(StringLiteral str, string char, float fraction) {
@@ -80,6 +81,11 @@ class HardcodedValueSource extends DataFlow::Node {
 
 class CredentialSink extends DataFlow::Node {
   CredentialSink() {
+    exists(string s | s.matches("credentials-%") |
+      // Actual sink-type will be things like `credentials-password` or `credentials-username`
+      this = ModelOutput::getASinkNode(s).asSink()
+    )
+    or
     exists(string name |
       name.regexpMatch(getACredentialRegex()) and
       not name.matches("%file")

python/ql/src/change-notes/2024-08-26-Cors-misconfiguration-middleware.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* The `py/cors-misconfiguration-with-credentials` query, which finds insecure CORS middleware configurations.

python/ql/src/change-notes/released/1.0.2.md

@@ -0,0 +1,3 @@
+## 1.0.2
+
+No user-facing changes.

python/ql/src/change-notes/released/1.0.3.md

@@ -1,4 +1,6 @@
----
-category: minorAnalysis
----
-* Additional sanitizers have been added to the `py/full-ssrf` and `py/partial-ssrf` queries for methods that verify a string contains only a certain set of characters, such as `.isalnum()` as well as regular expression tests.
+## 1.0.3
+
+### Minor Analysis Improvements
+
+* Adding Python support for Hardcoded Credentials as Models as Data
+* Additional sanitizers have been added to the `py/full-ssrf` and `py/partial-ssrf` queries for methods that verify a string contains only a certain set of characters, such as `.isalnum()` as well as regular expression tests.

python/ql/src/change-notes/released/1.0.4.md

@@ -0,0 +1,3 @@
+## 1.0.4
+
+No user-facing changes.

python/ql/src/change-notes/released/1.1.0.md

@@ -0,0 +1,9 @@
+## 1.1.0
+
+### New Queries
+
+* The `py/cookie-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack. This query finds instances of cookies being constructed from user input. 
+
+### Minor Analysis Improvements
+
+* Added models of `streamlit` PyPI package.

python/ql/src/change-notes/released/1.2.0.md

@@ -0,0 +1,5 @@
+## 1.2.0
+
+### New Queries
+
+* The `py/cookie-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack. This query finds instances of cookies being set without the `Secure`, `HttpOnly`, or `SameSite` attributes set to secure values.

python/ql/src/change-notes/released/1.2.1.md

@@ -0,0 +1,3 @@
+## 1.2.1
+
+No user-facing changes.

python/ql/src/change-notes/released/1.2.2.md

@@ -0,0 +1,5 @@
+## 1.2.2
+
+### Minor Analysis Improvements
+
+* The `py/clear-text-logging-sensitive-data` and `py/clear-text-storage-sensitive-data` queries have been updated to exclude the `certificate` classification of sensitive sources, which often do not contain sensitive data.

python/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.0.1
+lastReleaseVersion: 1.2.2

python/ql/src/experimental/Security/CWE-074/TemplateInjectionCustomizations.qll

@@ -45,7 +45,10 @@ module TemplateInjection {
   }
 
   /**
-   * A comparison with a constant string, considered as a sanitizer-guard.
+   * A comparison with a constant, considered as a sanitizer-guard.
    */
-  class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
+  class ConstCompareAsSanitizerGuard extends Sanitizer, ConstCompareBarrier { }
+
+  /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
+  deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
 }

python/ql/src/experimental/Security/CWE-091/XsltInjectionCustomizations.qll

@@ -52,7 +52,10 @@ module XsltInjection {
   }
 
   /**
-   * A comparison with a constant string, considered as a sanitizer-guard.
+   * A comparison with a constant, considered as a sanitizer-guard.
    */
-  class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
+  class ConstCompareAsSanitizerGuard extends Sanitizer, ConstCompareBarrier { }
+
+  /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
+  deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
 }

python/ql/src/experimental/Security/CWE-094/Js2Py.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+            Passing untrusted inputs to a JavaScript interpreter like `Js2Py` can lead to arbitrary
+            code execution.
+        </p>
+    </overview>
+    <recommendation>
+        <p> This vulnerability can be prevented either by preventing an untrusted user input to flow
+            to an <code>eval_js</code> call. Or, the impact of this vulnerability can be
+            significantly reduced by disabling imports from the interepreted code (note that in a <a
+                href="https://github.com/PiotrDabkowski/Js2Py/issues/45#issuecomment-258724406">
+            comment</a> the author of the library highlights that Js2Py is still insecure with this
+            option).</p>
+    </recommendation>
+    <example>
+        <p>In the example below, the Javascript code being evaluated is controlled by the user and
+            hence leads to arbitrary code execution.</p>
+        <sample src="Js2pyBad.py" />
+        <p>This can be fixed by disabling imports before evaluating the user passed buffer.</p>
+        <sample src="Js2pyGood.py" />
+    </example>
+</qhelp>

python/ql/src/experimental/Security/CWE-094/Js2Py.ql

@@ -0,0 +1,38 @@
+/**
+ * @name JavaScript code execution.
+ * @description Passing user supplied arguments to a Javascript to Python translation engine such as Js2Py can lead to remote code execution.
+ * @problem.severity error
+ * @security-severity 9.3
+ * @precision high
+ * @kind path-problem
+ * @id py/js2py-rce
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-94
+ */
+
+import python
+import semmle.python.ApiGraphs
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.Concepts
+
+module Js2PyFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    API::moduleImport("js2py").getMember(["eval_js", "eval_js6", "EvalJs"]).getACall().getArg(_) =
+      node
+  }
+}
+
+module Js2PyFlow = TaintTracking::Global<Js2PyFlowConfig>;
+
+import Js2PyFlow::PathGraph
+
+from Js2PyFlow::PathNode source, Js2PyFlow::PathNode sink
+where
+  Js2PyFlow::flowPath(source, sink) and
+  not exists(API::moduleImport("js2py").getMember("disable_pyimport").getACall())
+select sink, source, sink, "This input to Js2Py depends on a $@.", source.getNode(),
+  "user-provided value"

python/ql/src/experimental/Security/CWE-094/Js2pyBad.py

@@ -0,0 +1,4 @@
+@bp.route("/bad")
+def bad():
+    jk = flask.request.form["jk"]
+    jk = eval_js(f"{jk} f()")

python/ql/src/experimental/Security/CWE-094/Js2pyGood.py

@@ -0,0 +1,6 @@
+@bp.route("/good")
+def good():
+    # disable python imports to prevent execution of malicious code 
+    js2py.disable_pyimport()
+    jk = flask.request.form["jk"]
+    jk = eval_js(f"{jk} f()")

python/ql/src/experimental/Security/CWE-346/CorsBad.py

@@ -0,0 +1,9 @@
+import cherrypy
+
+def bad():
+    request = cherrypy.request
+    validCors = "domain.com"
+    if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
+        origin = request.headers.get('Origin', None)
+        if origin.startswith(validCors):
+            print("Origin Valid")

python/ql/src/experimental/Security/CWE-346/CorsBypass.qhelp

@@ -0,0 +1,28 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>Cross-origin resource sharing policy may be bypassed due to incorrect checks like the <code>string.startswith</code> call.</p>
+  </overview>
+  <recommendation>
+    <p>Use a more stronger check to test for CORS policy bypass.</p>
+  </recommendation>
+
+  <example>
+    <p>Most Python frameworks provide a mechanism for testing origins and performing CORS checks.
+      For example, consider the code snippet below, <code>origin</code> is compared using a <code>
+      startswith</code> call against a list of whitelisted origins. This check can be bypassed
+      easily by origin like <code>domain.com.baddomain.com</code>
+    </p>
+    <sample src="CorsBad.py" />
+    <p>This can be prevented by comparing the origin in a manner shown below.
+    </p>
+    <sample src="CorsGood.py" />
+
+  </example>
+
+  <references>
+    <li>PortsSwigger : <a href="https://portswigger.net/web-security/cors"></a>Cross-origin resource
+      sharing (CORS)</li>
+    <li>Related CVE: <a href="https://github.com/advisories/GHSA-824x-jcxf-hpfg">CVE-2022-3457</a>.</li>
+  </references>
+</qhelp>

python/ql/src/experimental/Security/CWE-346/CorsBypass.ql

@@ -0,0 +1,97 @@
+/**
+ * @name Cross Origin Resource Sharing(CORS) Policy Bypass
+ * @description Checking user supplied origin headers using weak comparators like 'string.startswith' may lead to CORS policy bypass.
+ * @kind path-problem
+ * @problem.severity warning
+ * @id py/cors-bypass
+ * @tags security
+ *       externa/cwe/CWE-346
+ */
+
+import python
+import semmle.python.ApiGraphs
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.Flow
+import semmle.python.dataflow.new.RemoteFlowSources
+
+/**
+ * Returns true if the control flow node may be useful in the current context.
+ *
+ * Ideally for more completeness, we should alert on every `startswith` call and every remote flow source which gets partailly checked. But, as this can lead to lots of FPs, we apply heuristics to filter some calls. This predicate provides logic for this filteration.
+ */
+private predicate maybeInteresting(ControlFlowNode c) {
+  // Check if the name of the variable which calls the function matches the heuristic.
+  // This would typically occur at the sink.
+  // This should deal with cases like
+  // `origin.startswith("bla")`
+  heuristics(c.(CallNode).getFunction().(AttrNode).getObject().(NameNode).getId())
+  or
+  // Check if the name of the variable passed as an argument to the functions matches the heuristic. This would typically occur at the sink.
+  // This should deal with cases like
+  // `bla.startswith(origin)`
+  heuristics(c.(CallNode).getArg(0).(NameNode).getId())
+  or
+  // Check if the value gets written to any interesting variable. This would typically occur at the source.
+  // This should deal with cases like
+  // `origin = request.headers.get('My-custom-header')`
+  exists(Variable v | heuristics(v.getId()) | c.getASuccessor*().getNode() = v.getAStore())
+}
+
+private class StringStartswithCall extends ControlFlowNode {
+  StringStartswithCall() { this.(CallNode).getFunction().(AttrNode).getName() = "startswith" }
+}
+
+bindingset[s]
+predicate heuristics(string s) { s.matches(["%origin%", "%cors%"]) }
+
+/**
+ * A member of the `cherrypy.request` class taken as a `RemoteFlowSource`.
+ */
+class CherryPyRequest extends RemoteFlowSource::Range {
+  CherryPyRequest() {
+    this =
+      API::moduleImport("cherrypy")
+          .getMember("request")
+          .getMember([
+              "charset", "content_type", "filename", "fp", "name", "params", "headers", "length",
+            ])
+          .asSource()
+  }
+
+  override string getSourceType() { result = "cherrypy.request" }
+}
+
+module CorsBypassConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(StringStartswithCall s |
+      node.asCfgNode() = s.(CallNode).getArg(0) or
+      node.asCfgNode() = s.(CallNode).getFunction().(AttrNode).getObject()
+    )
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(API::CallNode c, API::Node n |
+      n = API::moduleImport("cherrypy").getMember("request").getMember("headers") and
+      c = n.getMember("get").getACall()
+    |
+      c.getReturn().asSource() = node2 and n.asSource() = node1
+    )
+  }
+}
+
+module CorsFlow = TaintTracking::Global<CorsBypassConfig>;
+
+import CorsFlow::PathGraph
+
+from CorsFlow::PathNode source, CorsFlow::PathNode sink
+where
+  CorsFlow::flowPath(source, sink) and
+  (
+    maybeInteresting(source.getNode().asCfgNode())
+    or
+    maybeInteresting(sink.getNode().asCfgNode())
+  )
+select sink, source, sink,
+  "Potentially incorrect string comparison which could lead to a CORS bypass."

python/ql/src/experimental/Security/CWE-346/CorsGood.py

@@ -0,0 +1,9 @@
+import cherrypy
+
+def good():
+    request = cherrypy.request
+    validOrigin = "domain.com"
+    if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
+        origin = request.headers.get('Origin', None)
+        if origin == validOrigin:
+            print("Origin Valid")

python/ql/src/experimental/Security/CWE-614/CookieInjection.qhelp

@@ -1,28 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-<overview>
-<p>Constructing cookies from user input may allow an attacker to perform a Cookie Poisoning attack. 
-It is possible, however, to perform other parameter-like attacks through cookie poisoning techniques, 
-such as SQL Injection, Directory Traversal, or Stealth Commanding, etc. Additionally, 
-cookie injection may relate to attempts to perform Access of Administrative Interface.
-</p>
-</overview>
-
-<recommendation>
-<p>Do not use raw user input to construct cookies.</p>
-</recommendation>
-
-<example>
-<p>This example shows two ways of adding a cookie to a Flask response. The first way uses <code>set_cookie</code>'s
-and the second sets a cookie's raw value through a header, both using user-supplied input.</p>
-<sample src="CookieInjection.py" />
-</example>
-
-<references>
-<li>Imperva: <a href="https://docs.imperva.com/bundle/on-premises-knowledgebase-reference-guide/page/cookie_injection.htm">Cookie injection</a>.</li>
-</references>
-
-</qhelp>

python/ql/src/experimental/Security/CWE-614/CookieInjection.ql

@@ -1,27 +0,0 @@
-/**
- * @name Construction of a cookie using user-supplied input.
- * @description Constructing cookies from user input may allow an attacker to perform a Cookie Poisoning attack.
- * @kind path-problem
- * @problem.severity error
- * @id py/cookie-injection
- * @tags security
- *       experimental
- *       external/cwe/cwe-614
- */
-
-// determine precision above
-import python
-import semmle.python.dataflow.new.DataFlow
-import experimental.semmle.python.Concepts
-import experimental.semmle.python.CookieHeader
-import experimental.semmle.python.security.injection.CookieInjection
-import CookieInjectionFlow::PathGraph
-
-from CookieInjectionFlow::PathNode source, CookieInjectionFlow::PathNode sink, string insecure
-where
-  CookieInjectionFlow::flowPath(source, sink) and
-  if exists(sink.getNode().(CookieSink))
-  then insecure = ",and its " + sink.getNode().(CookieSink).getFlag() + " flag is not properly set."
-  else insecure = "."
-select sink.getNode(), source, sink, "Cookie is constructed from a $@" + insecure, source.getNode(),
-  "user-supplied input"

python/ql/src/experimental/Security/CWE-614/InsecureCookie.py

@@ -1,15 +0,0 @@
-from flask import Flask, request, make_response, Response
-
-
-@app.route("/1")
-def true():
-    resp = make_response()
-    resp.set_cookie("name", value="value", secure=True)
-    return resp
-
-
-@app.route("/2")
-def flask_make_response():
-    resp = make_response("hello")
-    resp.headers['Set-Cookie'] = "name=value; Secure;"
-    return resp

python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql

@@ -1,31 +0,0 @@
-/**
- * @name Failure to use secure cookies
- * @description Insecure cookies may be sent in cleartext, which makes them vulnerable to
- *              interception.
- * @kind problem
- * @problem.severity error
- * @security-severity 5.0
- * @precision ???
- * @id py/insecure-cookie
- * @tags security
- *       experimental
- *       external/cwe/cwe-614
- */
-
-// TODO: determine precision above
-import python
-import semmle.python.dataflow.new.DataFlow
-import experimental.semmle.python.Concepts
-import experimental.semmle.python.CookieHeader
-
-from Cookie cookie, string alert
-where
-  not cookie.isSecure() and
-  alert = "secure"
-  or
-  not cookie.isHttpOnly() and
-  alert = "httponly"
-  or
-  not cookie.isSameSite() and
-  alert = "samesite"
-select cookie, "Cookie is added without the '" + alert + "' flag properly set."

python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp

@@ -0,0 +1,64 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+	Web browsers, by default, disallow cross-origin resource sharing via direct HTTP requests.
+	Still, to satisfy some needs that arose with the growth of the web, an expedient was created to make exceptions possible.
+	CORS (Cross-origin resource sharing) is a mechanism that allows resources of a web endpoint (let's call it "Peer A")
+	to be accessed from another web page belonging to a different domain ("Peer B").
+</p>
+<p>
+	For that to happen, Peer A needs to make available its CORS configuration via special headers on the desired endpoint
+	via the OPTIONS method.
+</p>
+<p>
+	This configuration can also allow the inclusion of cookies on the cross-origin request,
+	(i.e. when the <code>Access-Control-Allow-Credentials</code> header is set to true) 
+	meaning that Peer B can send a request to Peer A that will include the cookies as if the request was executed by the user.
+</p>
+<p>
+	That can have dangerous effects if the origin of Peer B is not restricted correctly. 
+	An example of a dangerous scenario is when <code>Access-Control-Allow-Origin</code> header is set to a value obtained from the request made by Peer B 
+	(and not correctly validated), or is set to special values such as <code>*</code> or <code>null</code>. 
+	The above values can allow any Peer B to send requests to the misconfigured Peer A on behalf of the user.
+</p>
+<p>
+	Example scenario:
+	User is client of a bank that has its API misconfigured to accept CORS requests from any domain.
+	When the user loads an evil page, the evil page sends a request to the bank's API to transfer all funds
+	to evil party's account.
+	Given that the user was already logged in to the bank website, and had its session cookies set,
+	the evil party's request succeeds.
+</p>
+</overview>
+<recommendation>
+<p>
+	When configuring CORS that allow credentials passing,
+	it's best not to use user-provided values for the allowed origins response header,
+	especially if the cookies grant session permissions on the user's account.
+</p>
+<p>
+	It also can be very dangerous to set the allowed origins to <code>null</code> (which can be bypassed).
+</p>
+</recommendation>
+<example>
+<p>
+	The first example shows a possible CORS misconfiguration case:
+</p>
+<sample src="CorsMisconfigurationMiddlewareBad.py"/>
+<p>
+	The second example shows a better configuration:
+</p>
+<sample src="CorsMisconfigurationMiddlewareGood.py"/>
+</example>
+<references>
+<li>
+	Reference 1: <a href="https://portswigger.net/web-security/cors">PortSwigger Web Security Academy on CORS</a>.
+</li>
+<li>
+	Reference 2: <a href="https://www.youtube.com/watch?v=wgkj4ZgxI4c">AppSec EU 2017 Exploiting CORS Misconfigurations For Bitcoins And Bounties by James Kettle</a>.
+</li>
+</references>
+</qhelp>

python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql

@@ -0,0 +1,39 @@
+/**
+ * @name Cors misconfiguration with credentials
+ * @description Disabling or weakening SOP protection may make the application
+ *              vulnerable to a CORS attack.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 8.8
+ * @precision high
+ * @id py/cors-misconfiguration-with-credentials
+ * @tags security
+ *       external/cwe/cwe-942
+ */
+
+import python
+import semmle.python.Concepts
+private import semmle.python.dataflow.new.DataFlow
+
+predicate containsStar(DataFlow::Node array) {
+  array.asExpr() instanceof List and
+  array.asExpr().getASubExpression().(StringLiteral).getText() in ["*", "null"]
+  or
+  array.asExpr().(StringLiteral).getText() in ["*", "null"]
+}
+
+predicate isCorsMiddleware(Http::Server::CorsMiddleware middleware) {
+  middleware.getMiddlewareName() = "CORSMiddleware"
+}
+
+predicate credentialsAllowed(Http::Server::CorsMiddleware middleware) {
+  middleware.getCredentialsAllowed().asExpr() instanceof True
+}
+
+from Http::Server::CorsMiddleware a
+where
+  credentialsAllowed(a) and
+  containsStar(a.getOrigins().getALocalSource()) and
+  isCorsMiddleware(a)
+select a,
+  "This CORS middleware uses a vulnerable configuration that allows arbitrary websites to make authenticated cross-site requests"

python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py

@@ -0,0 +1,21 @@
+from fastapi import FastAPI
+from fastapi.middleware.cors import CORSMiddleware
+
+app = FastAPI()
+
+origins = [
+    "*"
+]
+
+app.add_middleware(
+    CORSMiddleware,
+    allow_origins=origins,
+    allow_credentials=True,
+    allow_methods=["*"],
+    allow_headers=["*"],
+)
+
+
+@app.get("/")
+async def main():
+    return {"message": "Hello World"}

python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py

@@ -0,0 +1,24 @@
+from fastapi import FastAPI
+from fastapi.middleware.cors import CORSMiddleware
+
+app = FastAPI()
+
+origins = [
+    "http://localhost.tiangolo.com",
+    "https://localhost.tiangolo.com",
+    "http://localhost",
+    "http://localhost:8080",
+]
+
+app.add_middleware(
+    CORSMiddleware,
+    allow_origins=origins,
+    allow_credentials=True,
+    allow_methods=["*"],
+    allow_headers=["*"],
+)
+
+
+@app.get("/")
+async def main():
+    return {"message": "Hello World"}

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -188,9 +188,6 @@ module LdapBind {
      * Holds if the binding process use SSL.
      */
     abstract predicate useSsl();
-
-    /** DEPRECATED: Alias for useSsl */
-    deprecated predicate useSSL() { this.useSsl() }
   }
 }
 
@@ -215,9 +212,6 @@ class LdapBind extends DataFlow::Node instanceof LdapBind::Range {
    * Holds if the binding process use SSL.
    */
   predicate useSsl() { super.useSsl() }
-
-  /** DEPRECATED: Alias for useSsl */
-  deprecated predicate useSSL() { this.useSsl() }
 }
 
 /** Provides classes for modeling SQL sanitization libraries. */
@@ -278,55 +272,6 @@ class CsvWriter extends DataFlow::Node instanceof CsvWriter::Range {
   DataFlow::Node getAnInput() { result = super.getAnInput() }
 }
 
-/**
- * A data-flow node that sets a cookie in an HTTP response.
- *
- * Extend this class to refine existing API models. If you want to model new APIs,
- * extend `Cookie::Range` instead.
- */
-class Cookie extends Http::Server::CookieWrite instanceof Cookie::Range {
-  /**
-   * Holds if this cookie is secure.
-   */
-  predicate isSecure() { super.isSecure() }
-
-  /**
-   * Holds if this cookie is HttpOnly.
-   */
-  predicate isHttpOnly() { super.isHttpOnly() }
-
-  /**
-   * Holds if the cookie is SameSite
-   */
-  predicate isSameSite() { super.isSameSite() }
-}
-
-/** Provides a class for modeling new cookie writes on HTTP responses. */
-module Cookie {
-  /**
-   * A data-flow node that sets a cookie in an HTTP response.
-   *
-   * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `Cookie` instead.
-   */
-  abstract class Range extends Http::Server::CookieWrite::Range {
-    /**
-     * Holds if this cookie is secure.
-     */
-    abstract predicate isSecure();
-
-    /**
-     * Holds if this cookie is HttpOnly.
-     */
-    abstract predicate isHttpOnly();
-
-    /**
-     * Holds if the cookie is SameSite.
-     */
-    abstract predicate isSameSite();
-  }
-}
-
 /** Provides classes for modeling JWT encoding-related APIs. */
 module JwtEncoding {
   /**

python/ql/src/experimental/semmle/python/CookieHeader.qll

@@ -1,76 +0,0 @@
-/**
- * Temporary: provides a class to extend current cookies to header declarations
- */
-
-import python
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import experimental.semmle.python.Concepts
-import semmle.python.Concepts
-
-/**
- * Gets a header setting a cookie.
- *
- * Given the following example:
- *
- * ```py
- * @app.route("/")
- * def flask_make_response():
- *    resp = make_response("")
- *    resp.headers['Set-Cookie'] = "name=value; Secure;"
- *    return resp
- * ```
- *
- * * `this` would be `resp.headers['Set-Cookie'] = "name=value; Secure;"`.
- * * `isSecure()` predicate would succeed.
- * * `isHttpOnly()` predicate would fail.
- * * `isSameSite()` predicate would fail.
- * * `getName()` and `getValue()` results would be `"name=value; Secure;"`.
- */
-class CookieHeader extends Cookie::Range instanceof Http::Server::ResponseHeaderWrite {
-  CookieHeader() {
-    exists(StringLiteral str |
-      str.getText() = "Set-Cookie" and
-      DataFlow::exprNode(str)
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getNameArg())
-    )
-  }
-
-  override predicate isSecure() {
-    exists(StringLiteral str |
-      str.getText().regexpMatch(".*; *Secure;.*") and
-      DataFlow::exprNode(str)
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
-    )
-  }
-
-  override predicate isHttpOnly() {
-    exists(StringLiteral str |
-      str.getText().regexpMatch(".*; *HttpOnly;.*") and
-      DataFlow::exprNode(str)
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
-    )
-  }
-
-  override predicate isSameSite() {
-    exists(StringLiteral str |
-      str.getText().regexpMatch(".*; *SameSite=(Strict|Lax);.*") and
-      DataFlow::exprNode(str)
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
-    )
-  }
-
-  override DataFlow::Node getNameArg() {
-    result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
-  }
-
-  override DataFlow::Node getValueArg() {
-    result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
-  }
-
-  override DataFlow::Node getHeaderArg() { none() }
-}

python/ql/src/experimental/semmle/python/Frameworks.qll

@@ -4,7 +4,6 @@
 
 private import experimental.semmle.python.frameworks.AsyncSsh
 private import experimental.semmle.python.frameworks.Stdlib
-private import experimental.semmle.python.frameworks.Flask
 private import experimental.semmle.python.frameworks.Django
 private import experimental.semmle.python.frameworks.LDAP
 private import experimental.semmle.python.frameworks.Netmiko

python/ql/src/experimental/semmle/python/frameworks/Django.qll

@@ -87,63 +87,6 @@ private module ExperimentalPrivateDjango {
             or
             result = baseClassRef().getReturn().getAMember()
           }
-
-          /**
-           * Gets a call to `set_cookie()`.
-           *
-           * Given the following example:
-           *
-           * ```py
-           * def django_response(request):
-           *    resp = django.http.HttpResponse()
-           *    resp.set_cookie("name", "value", secure=True, httponly=True, samesite='Lax')
-           *    return resp
-           * ```
-           *
-           * * `this` would be `resp.set_cookie("name", "value", secure=False, httponly=False, samesite='None')`.
-           * * `getName()`'s result would be `"name"`.
-           * * `getValue()`'s result would be `"value"`.
-           * * `isSecure()` predicate would succeed.
-           * * `isHttpOnly()` predicate would succeed.
-           * * `isSameSite()` predicate would succeed.
-           */
-          class DjangoResponseSetCookieCall extends DataFlow::MethodCallNode, Cookie::Range {
-            DjangoResponseSetCookieCall() {
-              this.calls(PrivateDjango::DjangoImpl::DjangoHttp::Response::HttpResponse::instance(),
-                "set_cookie")
-            }
-
-            override DataFlow::Node getNameArg() {
-              result in [this.getArg(0), this.getArgByName("key")]
-            }
-
-            override DataFlow::Node getValueArg() {
-              result in [this.getArg(1), this.getArgByName("value")]
-            }
-
-            override predicate isSecure() {
-              DataFlow::exprNode(any(True t))
-                  .(DataFlow::LocalSourceNode)
-                  .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
-            }
-
-            override predicate isHttpOnly() {
-              DataFlow::exprNode(any(True t))
-                  .(DataFlow::LocalSourceNode)
-                  .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
-            }
-
-            override predicate isSameSite() {
-              exists(StringLiteral str |
-                str.getText() in ["Strict", "Lax"] and
-                DataFlow::exprNode(str)
-                    .(DataFlow::LocalSourceNode)
-                    .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
-              )
-            }
-
-            override DataFlow::Node getHeaderArg() { none() }
-          }
         }
       }
     }

python/ql/src/experimental/semmle/python/frameworks/Flask.qll

@@ -1,62 +0,0 @@
-/**
- * Provides classes modeling security-relevant aspects of the `flask` PyPI package.
- * See https://flask.palletsprojects.com/en/1.1.x/.
- */
-
-private import python
-private import semmle.python.frameworks.Flask
-private import semmle.python.dataflow.new.DataFlow
-private import experimental.semmle.python.Concepts
-private import semmle.python.ApiGraphs
-private import semmle.python.frameworks.Flask
-
-module ExperimentalFlask {
-  /**
-   * Gets a call to `set_cookie()`.
-   *
-   * Given the following example:
-   *
-   * ```py
-   * @app.route("/")
-   * def false():
-   *    resp = make_response()
-   *    resp.set_cookie("name", value="value", secure=True, httponly=True, samesite='Lax')
-   *    return resp
-   * ```
-   *
-   * * `this` would be `resp.set_cookie("name", value="value", secure=False, httponly=False, samesite='None')`.
-   * * `getName()`'s result would be `"name"`.
-   * * `getValue()`'s result would be `"value"`.
-   * * `isSecure()` predicate would succeed.
-   * * `isHttpOnly()` predicate would succeed.
-   * * `isSameSite()` predicate would succeed.
-   */
-  class FlaskSetCookieCall extends Cookie::Range instanceof Flask::FlaskResponseSetCookieCall {
-    override DataFlow::Node getNameArg() { result = this.getNameArg() }
-
-    override DataFlow::Node getValueArg() { result = this.getValueArg() }
-
-    override predicate isSecure() {
-      DataFlow::exprNode(any(True t))
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
-    }
-
-    override predicate isHttpOnly() {
-      DataFlow::exprNode(any(True t))
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
-    }
-
-    override predicate isSameSite() {
-      exists(StringLiteral str |
-        str.getText() in ["Strict", "Lax"] and
-        DataFlow::exprNode(str)
-            .(DataFlow::LocalSourceNode)
-            .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
-      )
-    }
-
-    override DataFlow::Node getHeaderArg() { none() }
-  }
-}

python/ql/src/experimental/semmle/python/security/dataflow/EmailXss.qll

@@ -19,7 +19,7 @@ private module EmailXssConfig implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node sanitizer) {
     sanitizer = any(HtmlEscaping esc).getOutput()
     or
-    sanitizer instanceof StringConstCompareBarrier
+    sanitizer instanceof ConstCompareBarrier
   }
 
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {

python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll

@@ -1,41 +0,0 @@
-import python
-import experimental.semmle.python.Concepts
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import semmle.python.dataflow.new.RemoteFlowSources
-
-class CookieSink extends DataFlow::Node {
-  string flag;
-
-  CookieSink() {
-    exists(Cookie cookie |
-      this in [cookie.getNameArg(), cookie.getValueArg()] and
-      (
-        not cookie.isSecure() and
-        flag = "secure"
-        or
-        not cookie.isHttpOnly() and
-        flag = "httponly"
-        or
-        not cookie.isSameSite() and
-        flag = "samesite"
-      )
-    )
-  }
-
-  string getFlag() { result = flag }
-}
-
-/**
- * A taint-tracking configuration for detecting Cookie injections.
- */
-private module CookieInjectionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(Cookie c | sink in [c.getNameArg(), c.getValueArg()])
-  }
-}
-
-/** Global taint-tracking for detecting "Cookie injections" vulnerabilities. */
-module CookieInjectionFlow = TaintTracking::Global<CookieInjectionConfig>;

python/ql/src/experimental/semmle/python/security/injection/CsvInjection.qll

@@ -15,7 +15,7 @@ private module CsvInjectionConfig implements DataFlow::ConfigSig {
 
   predicate isBarrier(DataFlow::Node node) {
     node = DataFlow::BarrierGuard<startsWithCheck/3>::getABarrierNode() or
-    node instanceof StringConstCompareBarrier
+    node instanceof ConstCompareBarrier
   }
 }
 

python/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-queries
-version: 1.0.2-dev
+version: 1.2.3-dev
 groups:
   - python
   - queries

python/ql/src/utils/modeleditor/FrameworkModeEndpoints.ql

@@ -0,0 +1,14 @@
+/**
+ * @name Fetch endpoints for use in the model editor (framework mode)
+ * @description A list of endpoints accessible (methods and attributes) for consumers of the library. Excludes test and generated code.
+ * @kind table
+ * @id py/utils/modeleditor/framework-mode-endpoints
+ * @tags modeleditor endpoints framework-mode
+ */
+
+import modeling.ModelEditor
+
+from Endpoint endpoint
+select endpoint, endpoint.getNamespace(), endpoint.getClass(), endpoint.getFunctionName(),
+  endpoint.getParameters(), endpoint.getSupportedStatus(), endpoint.getFileName(),
+  endpoint.getSupportedType(), endpoint.getKind()