Merge pull request #20330 from michaelnebel/python/ql4ql

Python: Fix some Ql4Ql violations.

Author: michaelnebel

python/ql/lib/analysis/DefinitionTracking.qll

@@ -83,7 +83,7 @@ private predicate ssa_phi_defn(PhiFunction phi, Definition defn) {
   ssa_variable_defn(phi.getAnInput(), defn)
 }
 
-/** Holds if the ESSA defn `def`  refers to (`value`, `cls`, `origin`) given the context `context`. */
+/** Holds if the ESSA defn `def` refers to (`value`, `cls`, `origin`) given the context `context`. */
 private predicate ssa_defn_defn(EssaDefinition def, Definition defn) {
   ssa_phi_defn(def, defn)
   or

python/ql/lib/experimental/cryptography/CryptoArtifact.qll

@@ -95,7 +95,7 @@ abstract class CryptographicAlgorithm extends CryptographicArtifact {
   /**
    * Normalizes a raw name into a normalized name as found in `CryptoAlgorithmNames.qll`.
    * Subclassess should override for more api-specific normalization.
-   * By deafult, converts a raw name to upper-case with no hyphen, underscore, hash, or space.
+   * By default, converts a raw name to upper-case with no hyphen, underscore, hash, or space.
    */
   bindingset[s]
   string normalizeName(string s) {

python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll

@@ -117,31 +117,25 @@ module KDF {
     override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] }
 
     override DataFlow::Node getIterationSizeSrc() {
-      if this.requiresIteration()
-      then
-        // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC
-        result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations"))
-      else none()
+      this.requiresIteration() and
+      // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC
+      result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations"))
     }
 
     override DataFlow::Node getSaltConfigSrc() {
-      if this.requiresSalt()
-      then
-        // SCRYPT has it in arg 1
-        if this.getAlgorithm().getKDFName() = "SCRYPT"
-        then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt"))
-        else
-          // EVERYTHING ELSE that uses salt is in arg 2
-          result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "salt"))
-      else none()
+      this.requiresSalt() and
+      // SCRYPT has it in arg 1
+      if this.getAlgorithm().getKDFName() = "SCRYPT"
+      then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt"))
+      else
+        // EVERYTHING ELSE that uses salt is in arg 2
+        result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "salt"))
     }
 
     override DataFlow::Node getHashConfigSrc() {
-      if this.requiresHash()
-      then
-        // ASSUMPTION: ONLY EVER in arg 0
-        result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm"))
-      else none()
+      this.requiresHash() and
+      // ASSUMPTION: ONLY EVER in arg 0
+      result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm"))
     }
 
     // TODO: get encryption algorithm for CBC-based KDF?
@@ -152,11 +146,9 @@ module KDF {
     }
 
     override DataFlow::Node getModeSrc() {
-      if this.requiresMode()
-      then
-        // ASSUMPTION: ONLY EVER in arg 1
-        result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "mode"))
-      else none()
+      this.requiresMode() and
+      // ASSUMPTION: ONLY EVER in arg 1
+      result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "mode"))
     }
   }
 }

python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll

@@ -201,7 +201,7 @@ module KDF {
 
   // TODO: better modeling of scrypt
   /**
-   * Identifies key derivation fucntion hashlib.scrypt accesses.
+   * Identifies key derivation function hashlib.scrypt accesses.
    */
   class HashlibScryptAlgorithm extends KeyDerivationAlgorithm, KeyDerivationOperation {
     HashlibScryptAlgorithm() { this = API::moduleImport("hashlib").getMember("scrypt").getACall() }

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

@@ -613,7 +613,7 @@ DataFlowType getNodeType(Node node) {
 // Extra flow
 //--------
 /**
- * Holds if `pred` can flow to `succ`, by jumping from one callable to
+ * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to
  * another. Additional steps specified by the configuration are *not*
  * taken into account.
  */
@@ -634,7 +634,7 @@ predicate jumpStep(Node nodeFrom, Node nodeTo) {
  * the type-trackers as well, as that would make evaluation of type-tracking recursive
  * with the new jumpsteps.
  *
- * Holds if `pred` can flow to `succ`, by jumping from one callable to
+ * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to
  * another. Additional steps specified by the configuration are *not*
  * taken into account.
  */
@@ -657,7 +657,7 @@ predicate jumpStepSharedWithTypeTracker(Node nodeFrom, Node nodeTo) {
  * the type-trackers as well, as that would make evaluation of type-tracking recursive
  * with the new jumpsteps.
  *
- * Holds if `pred` can flow to `succ`, by jumping from one callable to
+ * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to
  * another. Additional steps specified by the configuration are *not*
  * taken into account.
  */
@@ -766,7 +766,7 @@ module Orm {
     abstract predicate storeStep(Node nodeFrom, Content c, Node nodeTo);
 
     /**
-     * Holds if `pred` can flow to `succ`, by jumping from one callable to
+     * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to
      * another. Additional steps specified by the configuration are *not*
      * taken into account.
      */

python/ql/lib/semmle/python/frameworks/Stdlib.qll

@@ -3702,11 +3702,8 @@ module StdlibPrivate {
    * A call to a find method on a tree or an element will execute an XPath expression.
    */
   private class ElementTreeFindCall extends XML::XPathExecution::Range, DataFlow::CallCfgNode {
-    string methodName;
-
     ElementTreeFindCall() {
-      methodName in ["find", "findall", "findtext"] and
-      (
+      exists(string methodName | methodName in ["find", "findall", "findtext"] |
         this = elementTreeInstance().getMember(methodName).getACall()
         or
         this = elementInstance().getMember(methodName).getACall()

python/ql/lib/semmle/python/objects/ObjectInternal.qll

@@ -174,9 +174,9 @@ class ObjectInternal extends TObject {
   abstract int length();
 
   /**
-   * Holds if the object `function` is called when this object is called and `paramOffset`
+   * Holds if the object `function` is called when this object is called and `offset`
    * is the difference from the parameter position and the argument position.
-   * For a normal function `paramOffset` is 0. For classes and bound-methods it is 1.
+   * For a normal function `offset` is 0. For classes and bound-methods it is 1.
    * This is used to implement the `CallableValue` public API.
    */
   predicate functionAndOffset(CallableObjectInternal function, int offset) { none() }

python/ql/lib/semmle/python/types/FunctionObject.qll

@@ -46,9 +46,7 @@ abstract class FunctionObject extends Object {
   ControlFlowNode getACall() { result = this.theCallable().getACall() }
 
   /** Gets a call-site from where this function is called, given the `context` */
-  ControlFlowNode getACall(Context caller_context) {
-    result = this.theCallable().getACall(caller_context)
-  }
+  ControlFlowNode getACall(Context context) { result = this.theCallable().getACall(context) }
 
   /**
    * Gets the `ControlFlowNode` that will be passed as the nth argument to `this` when called at `call`.

python/ql/src/Security/CWE-327/FluentApiModel.qll

@@ -15,7 +15,7 @@ import TlsLibraryModel
  * The state is represented as a bit vector, where each bit corresponds to a
  * protocol version. The bit is set if the protocol is allowed.
  */
-module InsecureContextConfiguration implements DataFlow::StateConfigSig {
+module InsecureContextConfig implements DataFlow::StateConfigSig {
   private newtype TFlowState =
     TMkFlowState(TlsLibrary library, int bits) {
       bits in [0 .. max(any(ProtocolVersion v).getBit()) * 2 - 1]
@@ -116,7 +116,12 @@ module InsecureContextConfiguration implements DataFlow::StateConfigSig {
   }
 }
 
-private module InsecureContextFlow = DataFlow::GlobalWithState<InsecureContextConfiguration>;
+/**
+ * DEPRECATED: Renamed to `InsecureContextConfig`.
+ */
+deprecated module InsecureContextConfiguration = InsecureContextConfig;
+
+private module InsecureContextFlow = DataFlow::GlobalWithState<InsecureContextConfig>;
 
 /**
  * Holds if `conectionCreation` marks the creation of a connection based on the contex

python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql

@@ -63,7 +63,7 @@ private module TarSlipImprovConfig implements DataFlow::ConfigSig {
       // For a call to `file.extractall` without `members` argument, `file` is considered a sink.
       exists(MethodCallNode call, AllTarfileOpens atfo |
         call = atfo.getReturn().getMember("extractall").getACall() and
-        not exists(Node arg | arg = call.getArgByName("members")) and
+        not exists(call.getArgByName("members")) and
         sink = call.getObject()
       )
       or