Merge branch 'main' of github.com:github/codeql into python-dataflow/flow-summaries-from-scratch

python/ql/src/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 0.4.2
+
+### New Queries
+
+* Added a new query, `py/suspicious-regexp-range`, to detect character ranges in regular expressions that seem to match 
+  too many characters.
+
 ## 0.4.1
 
 ## 0.4.0

python/ql/src/Exceptions/EmptyExcept.ql

@@ -12,6 +12,7 @@
  */
 
 import python
+import semmle.python.ApiGraphs
 
 predicate empty_except(ExceptStmt ex) {
   not exists(Stmt s | s = ex.getAStmt() and not s instanceof Pass)
@@ -28,7 +29,7 @@ predicate no_comment(ExceptStmt ex) {
 }
 
 predicate non_local_control_flow(ExceptStmt ex) {
-  ex.getType().pointsTo(ClassValue::stopIteration())
+  ex.getType() = API::builtin("StopIteration").getAValueReachableFromSource().asExpr()
 }
 
 predicate try_has_normal_exit(Try try) {
@@ -61,27 +62,32 @@ predicate subscript(Stmt s) {
   s.(Delete).getATarget() instanceof Subscript
 }
 
-predicate encode_decode(Call ex, ClassValue type) {
+predicate encode_decode(Call ex, Expr type) {
   exists(string name | ex.getFunc().(Attribute).getName() = name |
-    name = "encode" and type = ClassValue::unicodeEncodeError()
+    name = "encode" and
+    type = API::builtin("UnicodeEncodeError").getAValueReachableFromSource().asExpr()
     or
-    name = "decode" and type = ClassValue::unicodeDecodeError()
+    name = "decode" and
+    type = API::builtin("UnicodeDecodeError").getAValueReachableFromSource().asExpr()
   )
 }
 
-predicate small_handler(ExceptStmt ex, Stmt s, ClassValue type) {
+predicate small_handler(ExceptStmt ex, Stmt s, Expr type) {
   not exists(ex.getTry().getStmt(1)) and
   s = ex.getTry().getStmt(0) and
-  ex.getType().pointsTo(type)
+  ex.getType() = type
 }
 
 predicate focussed_handler(ExceptStmt ex) {
-  exists(Stmt s, ClassValue type | small_handler(ex, s, type) |
-    subscript(s) and type.getASuperType() = ClassValue::lookupError()
+  exists(Stmt s, Expr type | small_handler(ex, s, type) |
+    subscript(s) and
+    type = API::builtin("IndexError").getASubclass*().getAValueReachableFromSource().asExpr()
     or
-    attribute_access(s) and type = ClassValue::attributeError()
+    attribute_access(s) and
+    type = API::builtin("AttributeError").getAValueReachableFromSource().asExpr()
     or
-    s.(ExprStmt).getValue() instanceof Name and type = ClassValue::nameError()
+    s.(ExprStmt).getValue() instanceof Name and
+    type = API::builtin("NameError").getAValueReachableFromSource().asExpr()
     or
     encode_decode(s.(ExprStmt).getValue(), type)
   )

python/ql/src/Functions/ModificationOfParameterWithDefault.ql

@@ -19,5 +19,5 @@ from
   ModificationOfParameterWithDefault::Configuration config, DataFlow::PathNode source,
   DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ flows to here and is mutated.", source.getNode(),
-  "Default value"
+select sink.getNode(), source, sink, "This expression mutates $@.", source.getNode(),
+  "a default value"

python/ql/src/Lexical/CommentedOutCode.ql

@@ -16,4 +16,4 @@ import Lexical.CommentedOutCode
 
 from CommentedOutCodeBlock c
 where not c.maybeExampleCode()
-select c, "These comments appear to contain commented-out code."
+select c, "This comment appears to contain commented-out code."

python/ql/src/Security/CWE-022/TarSlip.ql

@@ -18,5 +18,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Extraction of tarfile from $@", source.getNode(),
+select sink.getNode(), source, sink, "This file extraction depends on $@", source.getNode(),
   "a potentially untrusted source"

python/ql/src/Security/CWE-078/CommandInjection.ql

@@ -20,5 +20,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
+select sink.getNode(), source, sink, "This command line depends on $@.", source.getNode(),
   "a user-provided value"

python/ql/src/Security/CWE-090/LdapInjection.ql

@@ -23,6 +23,5 @@ where
   or
   any(FilterConfiguration filterConfig).hasFlowPath(source, sink) and
   parameterName = "filter"
-select sink.getNode(), source, sink,
-  "$@ LDAP query parameter (" + parameterName + ") comes from $@.", sink.getNode(), "This",
-  source.getNode(), "a user-provided value"
+select sink.getNode(), source, sink, "$@ depends on $@.", sink.getNode(),
+  "LDAP query parameter (" + parameterName + ")", source.getNode(), "a user-provided value"

python/ql/src/Security/CWE-094/CodeInjection.ql

@@ -20,5 +20,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ flows to here and is interpreted as code.",
-  source.getNode(), "A user-provided value"
+select sink.getNode(), source, sink, "This code execution depends on $@.", source.getNode(),
+  "a user-provided value"

python/ql/src/Security/CWE-117/LogInjection.ql

@@ -17,5 +17,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ flows to log entry.", source.getNode(),
-  "User-provided value"
+select sink.getNode(), source, sink, "This log entry depends on $@.", source.getNode(),
+  "a user-provided value"

python/ql/src/Security/CWE-209/StackTraceExposure.ql

@@ -19,5 +19,6 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ may be exposed to an external user", source.getNode(),
-  "Error information"
+select sink.getNode(), source, sink,
+  "$@ flows to this location and may be exposed to an external user.", source.getNode(),
+  "Stack trace information"

python/ql/src/Security/CWE-295/RequestWithoutValidation.ql

@@ -13,7 +13,6 @@
 import python
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.Concepts
-private import semmle.python.ApiGraphs
 
 from
   HTTP::Client::Request request, DataFlow::Node disablingNode, DataFlow::Node origin, string ending

python/ql/src/Security/CWE-502/UnsafeDeserialization.ql

@@ -18,4 +18,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Deserializing of $@.", source.getNode(), "untrusted input"
+select sink.getNode(), source, sink, "Unsafe deserialization depends on $@.", source.getNode(),
+  "a user-provided value"

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

@@ -18,5 +18,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Untrusted URL redirection due to $@.", source.getNode(),
-  "A user-provided value"
+select sink.getNode(), source, sink, "Untrusted URL redirection depends on $@.", source.getNode(),
+  "a user-provided value"

python/ql/src/Security/CWE-611/Xxe.ql

@@ -19,5 +19,5 @@ import DataFlow::PathGraph
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
-  "A $@ is parsed as XML without guarding against external entity expansion.", source.getNode(),
-  "user-provided value"
+  "XML parsing depends on $@ without guarding against external entity expansion.", source.getNode(),
+  "a user-provided value"

python/ql/src/Security/CWE-643/XpathInjection.ql

@@ -17,4 +17,5 @@ import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink, source, sink, "This Xpath query depends on $@.", source, "a user-provided value"
+select sink.getNode(), source, sink, "XPath expression depends on $@.", source.getNode(),
+  "a user-provided value"

python/ql/src/Security/CWE-730/PolynomialReDoS.ql

@@ -4,6 +4,7 @@
  *              to match may be vulnerable to denial-of-service attacks.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 7.5
  * @precision high
  * @id py/polynomial-redos
  * @tags security

python/ql/src/Security/CWE-730/ReDoS.ql

@@ -5,6 +5,7 @@
  *              attacks.
  * @kind problem
  * @problem.severity error
+ * @security-severity 7.5
  * @precision high
  * @id py/redos
  * @tags security

python/ql/src/Security/CWE-730/RegexInjection.ql

@@ -5,6 +5,7 @@
  *              exponential time on certain inputs.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 7.5
  * @precision high
  * @id py/regex-injection
  * @tags security
@@ -23,6 +24,6 @@ from
 where
   config.hasFlowPath(source, sink) and
   regexExecution = sink.getNode().(Sink).getRegexExecution()
-select sink.getNode(), source, sink,
-  "$@ regular expression is constructed from a $@ and executed by $@.", sink.getNode(), "This",
-  source.getNode(), "user-provided value", regexExecution, regexExecution.getName()
+select sink.getNode(), source, sink, "$@ depends on $@ and executed by $@.", sink.getNode(),
+  "This regular expression", source.getNode(), "a user-provided value", regexExecution,
+  regexExecution.getName()

python/ql/src/Security/CWE-776/XmlBomb.ql

@@ -19,5 +19,5 @@ import DataFlow::PathGraph
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
-  "A $@ is parsed as XML without guarding against uncontrolled entity expansion.", source.getNode(),
-  "user-provided value"
+  "XML parsing depends on $@ without guarding against uncontrolled entity expansion.",
+  source.getNode(), "a user-provided value"

python/ql/src/Statements/UnreachableCode.ql

@@ -61,4 +61,4 @@ predicate reportable_unreachable(Stmt s) {
 
 from Stmt s
 where reportable_unreachable(s)
-select s, "Unreachable statement."
+select s, "This statement is unreachable."

python/ql/src/Variables/UnusedLocalVariable.ql

@@ -43,4 +43,4 @@ where
   unused_local(unused, v) and
   // If unused is part of a tuple, count it as unused if all elements of that tuple are unused.
   forall(Name el | el = unused.getParentNode().(Tuple).getAnElt() | unused_local(el, _))
-select unused, "The value assigned to local variable '" + v.getId() + "' is never used."
+select unused, "Variable " + v.getId() + " is not used"

python/ql/src/change-notes/2022-08-23-alert-messages.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The alert message of many queries have been changed to make the message consistent with other languages.

python/ql/src/change-notes/2022-08-29-missing-security-severity.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* Added the `security-severity` tag the `py/redos`, `py/polynomial-redos`, and `py/regex-injection` queries.

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

@@ -1,5 +1,6 @@
----
-category: newQuery
----
+## 0.4.2
+
+### New Queries
+
 * Added a new query, `py/suspicious-regexp-range`, to detect character ranges in regular expressions that seem to match 
   too many characters.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.1
+lastReleaseVersion: 0.4.2

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

@@ -22,11 +22,14 @@ private module ExperimentalPrivateDjango {
 
       module Request {
         module HttpRequest {
-          class DjangoGETParameter extends DataFlow::Node, RemoteFlowSource::Range {
-            DjangoGETParameter() { this = request().getMember("GET").getMember("get").getACall() }
+          class DjangoGetParameter extends DataFlow::Node, RemoteFlowSource::Range {
+            DjangoGetParameter() { this = request().getMember("GET").getMember("get").getACall() }
 
             override string getSourceType() { result = "django.http.request.GET.get" }
           }
+
+          /** DEPRECATED: Alias for DjangoGetParameter */
+          deprecated class DjangoGETParameter = DjangoGetParameter;
         }
       }
 

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

@@ -126,9 +126,9 @@ private module Ldap {
           (
             // ldap_connection.start_tls_s()
             // see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
-            exists(DataFlow::MethodCallNode startTLS |
-              startTLS.getObject().getALocalSource() = initialize and
-              startTLS.getMethodName() = "start_tls_s"
+            exists(DataFlow::MethodCallNode startTls |
+              startTls.getObject().getALocalSource() = initialize and
+              startTls.getMethodName() = "start_tls_s"
             )
             or
             // ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
@@ -234,9 +234,9 @@ private module Ldap {
         or
         // ldap_connection.start_tls_s()
         // see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
-        exists(DataFlow::MethodCallNode startTLS |
-          startTLS.getMethodName() = "start_tls_s" and
-          startTLS.getObject().getALocalSource() = this
+        exists(DataFlow::MethodCallNode startTls |
+          startTls.getMethodName() = "start_tls_s" and
+          startTls.getObject().getALocalSource() = this
         )
       }
 

python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll

@@ -31,8 +31,8 @@ module SmtpLib {
    * argument. Used because of the impossibility to get local source nodes from `_subparts`'
    * `(List|Tuple)` elements.
    */
-  private class SMTPMessageConfig extends TaintTracking2::Configuration {
-    SMTPMessageConfig() { this = "SMTPMessageConfig" }
+  private class SmtpMessageConfig extends TaintTracking2::Configuration {
+    SmtpMessageConfig() { this = "SMTPMessageConfig" }
 
     override predicate isSource(DataFlow::Node source) { source = mimeText(_) }
 
@@ -87,7 +87,7 @@ module SmtpLib {
         sink =
           [sendCall.getArg(2), sendCall.getArg(2).(DataFlow::MethodCallNode).getObject()]
               .getALocalSource() and
-        any(SMTPMessageConfig a)
+        any(SmtpMessageConfig a)
             .hasFlow(source, sink.(DataFlow::CallCfgNode).getArgByName("_subparts"))
         or
         // via .attach()
@@ -117,7 +117,7 @@ module SmtpLib {
    * * `sub` would be `message["Subject"]` (`Subscript`)
    * * `result` would be `"multipart test"`
    */
-  private DataFlow::Node getSMTPSubscriptByIndex(DataFlow::CallCfgNode sendCall, string index) {
+  private DataFlow::Node getSmtpSubscriptByIndex(DataFlow::CallCfgNode sendCall, string index) {
     exists(DefinitionNode def, Subscript sub |
       sub = def.getNode() and
       DataFlow::exprNode(sub.getObject()).getALocalSource() =
@@ -163,15 +163,15 @@ module SmtpLib {
     override DataFlow::Node getHtmlBody() { result = getSmtpMessage(this, "html") }
 
     override DataFlow::Node getTo() {
-      result in [this.getArg(1), getSMTPSubscriptByIndex(this, "To")]
+      result in [this.getArg(1), getSmtpSubscriptByIndex(this, "To")]
     }
 
     override DataFlow::Node getFrom() {
-      result in [this.getArg(0), getSMTPSubscriptByIndex(this, "From")]
+      result in [this.getArg(0), getSmtpSubscriptByIndex(this, "From")]
     }
 
     override DataFlow::Node getSubject() {
-      result in [this.getArg(2), getSMTPSubscriptByIndex(this, "Subject")]
+      result in [this.getArg(2), getSmtpSubscriptByIndex(this, "Subject")]
     }
   }
 }

python/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-queries
-version: 0.4.2-dev
+version: 0.4.3-dev
 groups: 
   - python
   - queries