fix conflict

python/ql/src/CHANGELOG.md

@@ -1,3 +1,25 @@
+## 1.0.1
+
+### Minor Analysis Improvements
+
+* Added models for `opml` library.
+
+## 1.0.0
+
+### Breaking Changes
+
+* CodeQL package management is now generally available, and all GitHub-produced CodeQL packages have had their version numbers increased to 1.0.0.
+
+### Minor Analysis Improvements
+
+* Added models of `gradio` PyPI package.
+
+## 0.9.16
+
+### New Queries
+
+* The `py/header-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack and renamed to `py/http-response-splitting`. This query finds instances of http header injection / response splitting vulnerabilities.
+
 ## 0.9.15
 
 No user-facing changes.

python/ql/src/Imports/UnusedImport.ql

@@ -12,6 +12,29 @@
 
 import python
 import Variables.Definition
+import semmle.python.ApiGraphs
+
+private predicate is_pytest_fixture(Import imp, Variable name) {
+  exists(Alias a, API::Node pytest_fixture, API::Node decorator |
+    pytest_fixture = API::moduleImport("pytest").getMember("fixture") and
+    // The additional `.getReturn()` is to account for the difference between
+    // ```
+    // @pytest.fixture
+    // def foo():
+    //    ...
+    // ```
+    // and
+    // ```
+    // @pytest.fixture(some, args, here)
+    // def foo():
+    //    ...
+    // ```
+    decorator in [pytest_fixture, pytest_fixture.getReturn()] and
+    a = imp.getAName() and
+    a.getAsname().(Name).getVariable() = name and
+    a.getValue() = decorator.getReturn().getAValueReachableFromSource().asExpr()
+  )
+}
 
 predicate global_name_used(Module m, string name) {
   exists(Name u, GlobalVariable v |
@@ -117,6 +140,7 @@ predicate unused_import(Import imp, Variable name) {
   not all_not_understood(imp.getEnclosingModule()) and
   not imported_module_used_in_doctest(imp) and
   not imported_alias_used_in_typehint(imp, name) and
+  not is_pytest_fixture(imp, name) and
   // Only consider import statements that actually point-to something (possibly an unknown module).
   // If this is not the case, it's likely that the import statement never gets executed.
   imp.getAName().getValue().pointsTo(_)

python/ql/src/Security/CWE-113/HeaderInjection.qhelp

@@ -0,0 +1,41 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Directly writing user input (for example, an HTTP request parameter) to an HTTP header
+can lead to an HTTP response-splitting vulnerability.</p>
+
+<p>If user-controlled input is used in an HTTP header that allows line break characters, an attacker can 
+inject additional headers or control the response body, leading to vulnerabilities such as XSS or cache poisoning.
+</p>
+
+</overview>
+
+<recommendation>
+<p>Ensure that user input containing line break characters is not written to an HTTP header.</p>
+</recommendation>
+
+<example>
+<p>In the following example, the case marked BAD writes user input to the header name. 
+In the GOOD case, input is first escaped to not contain any line break characters.</p>
+<sample src="examples/header_injection.py" />
+</example>
+
+<references>
+<li>
+SecLists.org: <a href="https://seclists.org/bugtraq/2005/Apr/187">HTTP response splitting</a>.
+</li>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/HTTP_Response_Splitting">HTTP Response Splitting</a>.
+</li>
+<li>
+Wikipedia: <a href="http://en.wikipedia.org/wiki/HTTP_response_splitting">HTTP response splitting</a>.
+</li>
+<li>
+CAPEC: <a href="https://capec.mitre.org/data/definitions/105.html">CAPEC-105: HTTP Request Splitting</a>
+</li>
+</references>
+</qhelp>

python/ql/src/Security/CWE-113/HeaderInjection.ql

@@ -1,19 +1,19 @@
 /**
- * @name HTTP Header Injection
- * @description User input should not be used in HTTP headers, otherwise a malicious user
- *              may be able to inject a value that could manipulate the response.
+ * @name HTTP Response Splitting
+ * @description Writing user input directly to an HTTP header
+ *              makes code vulnerable to attack by header splitting.
  * @kind path-problem
  * @problem.severity error
- * @id py/header-injection
+ * @security-severity 6.1
+ * @precision high
+ * @id py/http-response-splitting
  * @tags security
- *       experimental
  *       external/cwe/cwe-113
  *       external/cwe/cwe-079
  */
 
-// determine precision above
 import python
-import experimental.semmle.python.security.injection.HTTPHeaders
+import semmle.python.security.dataflow.HttpHeaderInjectionQuery
 import HeaderInjectionFlow::PathGraph
 
 from HeaderInjectionFlow::PathNode source, HeaderInjectionFlow::PathNode sink

python/ql/src/Security/CWE-113/examples/header_injection.py

@@ -0,0 +1,17 @@
+@app.route("/example_bad")
+def example_bad():
+    rfs_header = request.args["rfs_header"]
+    response = Response()
+    custom_header = "X-MyHeader-" + rfs_header
+    # BAD: User input is used as part of the header name.
+    response.headers[custom_header] = "HeaderValue" 
+    return response
+
+@app.route("/example_good")
+def example_bad():
+    rfs_header = request.args["rfs_header"]
+    response = Response()
+    custom_header = "X-MyHeader-" + rfs_header.replace("\n", "").replace("\r","").replace(":","")
+    # GOOD: Line break characters are removed from the input.
+    response.headers[custom_header] = "HeaderValue" 
+    return response

python/ql/src/Security/CWE-601/UrlRedirect.qhelp

@@ -45,12 +45,21 @@ attribute is empty.
 </p>
 
 <p>
-Note, however, that many browsers accept backslash characters (<code>\</code>) as equivalent
-to forward slash characters (<code>/</code>) in URLs, but the <code>urlparse</code> function
-does not. To account for this, you can first replace all backslashes with forward slashes,
-as shown in the following example:
+Note, however, that some cases are not handled as we desire out-of-the-box by <code>urlparse</code>, so we need to adjust two things, as shown in the example below:
 </p>
 
+<ul>
+  <li>
+    Many browsers accept backslash characters (<code>\</code>) as equivalent
+    to forward slash characters (<code>/</code>) in URLs, but the <code>urlparse</code> function
+    does not.
+  </li>
+  <li>
+    Mistyped URLs such as <code>https:/example.com</code> or <code>https:///example.com</code> are parsed as having an empty <code>netloc</code> attribute, while browsers will still redirect to the correct site.
+  </li>
+</ul>
+
+
 <sample src="examples/redirect_good2.py"/>
 
 <p>

python/ql/src/Security/CWE-601/examples/redirect_good2.py

@@ -7,7 +7,7 @@ app = Flask(__name__)
 def hello():
     target = request.args.get('target', '')
     target = target.replace('\\', '')
-    if not urlparse(target).netloc:
+    if not urlparse(target).netloc and not urlparse(target).scheme:
         # relative path, safe to redirect
         return redirect(target, code=302)
     # ignore the target and redirect to the home page

python/ql/src/change-notes/2024-06-04-ssrf-sanitizers.md

@@ -0,0 +1,4 @@
+---
+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.

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

@@ -0,0 +1,5 @@
+## 0.9.16
+
+### New Queries
+
+* The `py/header-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack and renamed to `py/http-response-splitting`. This query finds instances of http header injection / response splitting vulnerabilities.

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

@@ -0,0 +1,9 @@
+## 1.0.0
+
+### Breaking Changes
+
+* CodeQL package management is now generally available, and all GitHub-produced CodeQL packages have had their version numbers increased to 1.0.0.
+
+### Minor Analysis Improvements
+
+* Added models of `gradio` PyPI package.

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

@@ -0,0 +1,5 @@
+## 1.0.1
+
+### Minor Analysis Improvements
+
+* Added models for `opml` library.

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

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

python/ql/src/experimental/Security/CWE-113/HeaderInjection.qhelp

@@ -1,26 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>If an HTTP Header is built using string concatenation or string formatting, and the
-components of the concatenation include user input, a user 
-is likely to be able to manipulate the response.</p>
-</overview>
-
-<recommendation>
-<p>User input should not be included in an HTTP Header.</p>
-</recommendation>
-
-<example>
-<p>In the following example, the code appends a user-provided value into a header.</p>
-
-<sample src="header_injection.py" />
-</example>
-
-<references>
-<li>OWASP: <a href="https://owasp.org/www-community/attacks/HTTP_Response_Splitting">HTTP Response Splitting</a>.</li>
-<li>Python Security: <a href="https://python-security.readthedocs.io/vuln/http-header-injection.html">HTTP header injection</a>.</li>
-<li>SonarSource: <a href="https://rules.sonarsource.com/python/RSPEC-5167">RSPEC-5167</a>.</li>
-</references>
-</qhelp>

python/ql/src/experimental/Security/CWE-113/header_injection.py

@@ -1,9 +0,0 @@
-from flask import Response, request, Flask, make_response
-
-
-@app.route("/flask_Response")
-def flask_Response():
-    rfs_header = request.args["rfs_header"]
-    response = Response()
-    response.headers['HeaderName'] = rfs_header
-    return response

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

@@ -249,45 +249,6 @@ class SqlEscape extends DataFlow::Node instanceof SqlEscape::Range {
   DataFlow::Node getAnInput() { result = super.getAnInput() }
 }
 
-/** Provides classes for modeling HTTP Header APIs. */
-module HeaderDeclaration {
-  /**
-   * A data-flow node that collects functions setting HTTP Headers.
-   *
-   * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `HeaderDeclaration` instead.
-   */
-  abstract class Range extends DataFlow::Node {
-    /**
-     * Gets the argument containing the header name.
-     */
-    abstract DataFlow::Node getNameArg();
-
-    /**
-     * Gets the argument containing the header value.
-     */
-    abstract DataFlow::Node getValueArg();
-  }
-}
-
-/**
- * A data-flow node that collects functions setting HTTP Headers.
- *
- * Extend this class to refine existing API models. If you want to model new APIs,
- * extend `HeaderDeclaration::Range` instead.
- */
-class HeaderDeclaration extends DataFlow::Node instanceof HeaderDeclaration::Range {
-  /**
-   * Gets the argument containing the header name.
-   */
-  DataFlow::Node getNameArg() { result = super.getNameArg() }
-
-  /**
-   * Gets the argument containing the header value.
-   */
-  DataFlow::Node getValueArg() { result = super.getValueArg() }
-}
-
 /** Provides classes for modeling Csv writer APIs. */
 module CsvWriter {
   /**

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

@@ -6,6 +6,7 @@ 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.
@@ -26,13 +27,13 @@ import experimental.semmle.python.Concepts
  * * `isSameSite()` predicate would fail.
  * * `getName()` and `getValue()` results would be `"name=value; Secure;"`.
  */
-class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
+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.(HeaderDeclaration).getNameArg())
+          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getNameArg())
     )
   }
 
@@ -41,7 +42,7 @@ class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
       str.getText().regexpMatch(".*; *Secure;.*") and
       DataFlow::exprNode(str)
           .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(HeaderDeclaration).getValueArg())
+          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
     )
   }
 
@@ -50,7 +51,7 @@ class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
       str.getText().regexpMatch(".*; *HttpOnly;.*") and
       DataFlow::exprNode(str)
           .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(HeaderDeclaration).getValueArg())
+          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
     )
   }
 
@@ -59,13 +60,17 @@ class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
       str.getText().regexpMatch(".*; *SameSite=(Strict|Lax);.*") and
       DataFlow::exprNode(str)
           .(DataFlow::LocalSourceNode)
-          .flowsTo(this.(HeaderDeclaration).getValueArg())
+          .flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
     )
   }
 
-  override DataFlow::Node getNameArg() { result = this.(HeaderDeclaration).getValueArg() }
+  override DataFlow::Node getNameArg() {
+    result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
+  }
 
-  override DataFlow::Node getValueArg() { result = this.(HeaderDeclaration).getValueArg() }
+  override DataFlow::Node getValueArg() {
+    result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
+  }
 
   override DataFlow::Node getHeaderArg() { none() }
 }

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

@@ -6,7 +6,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.Werkzeug
 private import experimental.semmle.python.frameworks.LDAP
 private import experimental.semmle.python.frameworks.Netmiko
 private import experimental.semmle.python.frameworks.Paramiko

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

@@ -88,31 +88,6 @@ private module ExperimentalPrivateDjango {
             result = baseClassRef().getReturn().getAMember()
           }
 
-          class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-            DjangoResponseSetItemCall() {
-              this = baseClassRef().getReturn().getMember("__setitem__").getACall()
-            }
-
-            override DataFlow::Node getNameArg() { result = this.getArg(0) }
-
-            override DataFlow::Node getValueArg() { result = this.getArg(1) }
-          }
-
-          class DjangoResponseDefinition extends DataFlow::Node, HeaderDeclaration::Range {
-            DataFlow::Node headerInput;
-
-            DjangoResponseDefinition() {
-              headerInput = headerInstance().asSink() and
-              headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
-            }
-
-            override DataFlow::Node getNameArg() {
-              result.asExpr() = this.asExpr().(Subscript).getIndex()
-            }
-
-            override DataFlow::Node getValueArg() { result = headerInput }
-          }
-
           /**
            * Gets a call to `set_cookie()`.
            *

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

@@ -11,76 +11,6 @@ private import semmle.python.ApiGraphs
 private import semmle.python.frameworks.Flask
 
 module ExperimentalFlask {
-  /**
-   * A reference to either `flask.make_response` function, or the `make_response` method on
-   * an instance of `flask.Flask`. This creates an instance of the `flask_response`
-   * class (class-attribute on a flask application), which by default is
-   * `flask.Response`.
-   *
-   * See
-   * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.make_response
-   * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.make_response
-   */
-  private API::Node flaskMakeResponse() {
-    result =
-      [API::moduleImport("flask"), Flask::FlaskApp::instance()]
-          .getMember(["make_response", "jsonify", "make_default_options_response"])
-  }
-
-  /** Gets a reference to a header instance. */
-  private DataFlow::LocalSourceNode headerInstance() {
-    result =
-      [Flask::Response::classRef(), flaskMakeResponse()]
-          .getReturn()
-          .getAMember()
-          .getAValueReachableFromSource()
-  }
-
-  /** Gets a reference to a header instance call/subscript */
-  private DataFlow::Node headerInstanceCall() {
-    headerInstance() in [result.(DataFlow::AttrRead), result.(DataFlow::AttrRead).getObject()] or
-    headerInstance().asExpr() = result.asExpr().(Subscript).getObject()
-  }
-
-  class FlaskHeaderDefinition extends DataFlow::Node, HeaderDeclaration::Range {
-    DataFlow::Node headerInput;
-
-    FlaskHeaderDefinition() {
-      this.asCfgNode().(DefinitionNode) = headerInstanceCall().asCfgNode() and
-      headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
-    }
-
-    override DataFlow::Node getNameArg() { result.asExpr() = this.asExpr().(Subscript).getIndex() }
-
-    override DataFlow::Node getValueArg() { result = headerInput }
-  }
-
-  private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-    KeyValuePair item;
-
-    FlaskMakeResponseExtend() {
-      this.getFunction() = headerInstanceCall() and
-      item = this.getArg(_).asExpr().(Dict).getAnItem()
-    }
-
-    override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
-
-    override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
-  }
-
-  private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-    KeyValuePair item;
-
-    FlaskResponse() {
-      this = Flask::Response::classRef().getACall() and
-      item = this.getArg(_).asExpr().(Dict).getAnItem()
-    }
-
-    override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
-
-    override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
-  }
-
   /**
    * Gets a call to `set_cookie()`.
    *

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

@@ -1,33 +0,0 @@
-/**
- * Provides classes modeling security-relevant aspects of the `Werkzeug` PyPI package.
- * See
- * - https://pypi.org/project/Werkzeug/
- * - https://werkzeug.palletsprojects.com/en/1.0.x/#werkzeug
- */
-
-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 module Werkzeug {
-  module Datastructures {
-    module Headers {
-      class WerkzeugHeaderAddCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-        WerkzeugHeaderAddCall() {
-          this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
-            API::moduleImport("werkzeug")
-                .getMember("datastructures")
-                .getMember("Headers")
-                .getACall() and
-          this.getFunction().(DataFlow::AttrRead).getAttributeName() = "add"
-        }
-
-        override DataFlow::Node getNameArg() { result = this.getArg(0) }
-
-        override DataFlow::Node getValueArg() { result = this.getArg(1) }
-      }
-    }
-  }
-}

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

@@ -1,21 +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
-
-/**
- * A taint-tracking configuration for detecting HTTP Header injections.
- */
-private module HeaderInjectionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(HeaderDeclaration headerDeclaration |
-      sink in [headerDeclaration.getNameArg(), headerDeclaration.getValueArg()]
-    )
-  }
-}
-
-/** Global taint-tracking for detecting "HTTP Header injection" vulnerabilities. */
-module HeaderInjectionFlow = TaintTracking::Global<HeaderInjectionConfig>;

python/ql/src/qlpack.yml

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