Python: resolve remaining TODOs

github · Feb 6, 2025 · d3ee658 · d3ee658
1 parent 975ce06
commit d3ee658
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 50 deletions.
diff --git a/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll
@@ -20,12 +20,7 @@ private module LdapInjectionDnConfig implements DataFlow::ConfigSig {
 
   predicate isBarrier(DataFlow::Node node) { node instanceof DnSanitizer }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-090/LdapInjection.ql:26: Column 1 does not select a source or sink originating from the flow call on line 21
-    // ql/src/Security/CWE-090/LdapInjection.ql:27: Column 5 does not select a source or sink originating from the flow call on line 21
-    none()
-  }
+  predicate observeDiffInformedIncrementalMode() { any() }
 }
 
 /** Global taint-tracking for detecting "LDAP injection via the distinguished name (DN) parameter" vulnerabilities. */
@@ -38,12 +33,7 @@ private module LdapInjectionFilterConfig implements DataFlow::ConfigSig {
 
   predicate isBarrier(DataFlow::Node node) { node instanceof FilterSanitizer }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-090/LdapInjection.ql:26: Column 1 does not select a source or sink originating from the flow call on line 24
-    // ql/src/Security/CWE-090/LdapInjection.ql:27: Column 5 does not select a source or sink originating from the flow call on line 24
-    none()
-  }
+  predicate observeDiffInformedIncrementalMode() { any() }
 }
 
 /** Global taint-tracking for detecting "LDAP injection via the filter parameter" vulnerabilities. */

diff --git a/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll
@@ -19,10 +19,12 @@ private module RegexInjectionConfig implements DataFlow::ConfigSig {
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-730/RegexInjection.ql:29: Column 7 selects sink.getRegexExecution
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getRegexExecution().getLocation()
   }
 }
 

diff --git a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll
@@ -30,11 +30,12 @@ private module FullServerSideRequestForgeryConfig implements DataFlow::ConfigSig
     node instanceof FullUrlControlSanitizer
   }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll:47: Flow call outside 'select' clause
-    // ql/src/Security/CWE-918/FullServerSideRequestForgery.ql:24: Column 1 selects sink.getRequest
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getRequest().getLocation()
   }
 }
 
@@ -66,10 +67,12 @@ private module PartialServerSideRequestForgeryConfig implements DataFlow::Config
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-918/PartialServerSideRequestForgery.ql:24: Column 1 selects sink.getRequest
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getRequest().getLocation()
   }
 }
 

diff --git a/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll
@@ -29,11 +29,14 @@ module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigSig {
   // override to require the path doesn't have unmatched return steps
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql:27: Column 1 selects sink.getStringConstruction
-    // ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql:29: Column 7 selects sink.getCommandExecution
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getStringConstruction().getLocation()
+    or
+    result = sink.(Sink).getCommandExecution().getLocation()
   }
 }
 

diff --git a/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll b/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll
@@ -34,11 +34,7 @@ module NormalHashFunction {
       sensitiveDataExtraStepForCalls(node1, node2)
     }
 
-    predicate observeDiffInformedIncrementalMode() {
-      // TODO(diff-informed): Manually verify if config can be diff-informed.
-      // ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll:88: Flow call outside 'select' clause
-      none()
-    }
+    predicate observeDiffInformedIncrementalMode() { any() }
   }
 
   /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on sensitive data" vulnerabilities. */
@@ -70,11 +66,7 @@ module ComputationallyExpensiveHashFunction {
       sensitiveDataExtraStepForCalls(node1, node2)
     }
 
-    predicate observeDiffInformedIncrementalMode() {
-      // TODO(diff-informed): Manually verify if config can be diff-informed.
-      // ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll:95: Flow call outside 'select' clause
-      none()
-    }
+    predicate observeDiffInformedIncrementalMode() { any() }
   }
 
   /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on passwords" vulnerabilities. */

diff --git a/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll b/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll
@@ -173,10 +173,7 @@ private module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode }
 
   predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll:181: Flow call outside 'select' clause
-    // ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll:184: Flow call outside 'select' clause
-    none()
+    none() // Not used for PR analysis
   }
 }
 

diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll
@@ -112,9 +112,7 @@ module InsecureContextConfiguration implements DataFlow::StateConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/Security/CWE-327/FluentApiModel.qll:130: Flow call outside 'select' clause
-    none()
+    none() // Too complicated, but might be possible after some refactoring.
   }
 }
 

diff --git a/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll b/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll
@@ -40,9 +40,7 @@ module SmtpLib {
     }
 
     predicate observeDiffInformedIncrementalMode() {
-      // TODO(diff-informed): Manually verify if config can be diff-informed.
-      // ql/src/experimental/semmle/python/libraries/SmtpLib.qll:91: Flow call outside 'select' clause
-      none()
+      none() // Used in library model
     }
   }