Ruby: resolve inserted TODOs

Author: asgerf

ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll

@@ -18,10 +18,10 @@ private module Config 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/experimental/cwe-807/ConditionalBypass.ql:78: Flow call outside 'select' clause
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.getLocation() or result = sink.(Sink).getAction().getLocation()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll

@@ -21,10 +21,12 @@ private module InsecureDownloadConfig implements DataFlow::StateConfigSig {
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/queries/security/cwe-829/InsecureDownload.ql:20: Column 5 selects sink.getDownloadCall
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getDownloadCall().getLocation()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll

@@ -25,10 +25,12 @@ private module UnsafeCodeConstructionConfig 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/queries/security/cwe-094/UnsafeCodeConstruction.ql:25: Column 7 selects sink.getCodeSink
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getCodeSink().getLocation()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll

@@ -22,10 +22,12 @@ private module UnsafeHtmlConstructionConfig 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/queries/security/cwe-079/UnsafeHtmlConstruction.ql:24: Column 7 selects sink.getXssSink
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getXssSink().getLocation()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll

@@ -27,11 +27,14 @@ private module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigS
   // 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/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:26: Column 1 selects sink.getStringConstruction
-    // ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:28: 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()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll

@@ -29,11 +29,7 @@ module NormalHashFunction {
 
     predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
-    predicate observeDiffInformedIncrementalMode() {
-      // TODO(diff-informed): Manually verify if config can be diff-informed.
-      // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:83: 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. */
@@ -61,11 +57,7 @@ module ComputationallyExpensiveHashFunction {
 
     predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
-    predicate observeDiffInformedIncrementalMode() {
-      // TODO(diff-informed): Manually verify if config can be diff-informed.
-      // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:90: 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. */

ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll

@@ -18,11 +18,14 @@ private module MissingFullAnchorConfig 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/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 7 selects sink.getCallNode
-    // ql/src/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 9 selects sink.getRegex
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getCallNode().getLocation()
+    or
+    result = sink.(Sink).getRegex().getLocation()
   }
 }
 

ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll

@@ -19,11 +19,14 @@ private module PolynomialReDoSConfig 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/queries/security/cwe-1333/PolynomialReDoS.ql:27: Column 1 selects sink.getHighlight
-    // ql/src/queries/security/cwe-1333/PolynomialReDoS.ql:29: Column 5 selects sink.getRegExp
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getHighlight().getLocation()
+    or
+    result = sink.(Sink).getRegExp().getLocation()
   }
 }
 

ruby/ql/src/experimental/decompression-api/DecompressionApi.ql

@@ -40,10 +40,12 @@ private module DecompressionApiConfig implements DataFlow::ConfigSig {
   // our Decompression APIs defined above will be the sinks we use for this query
   predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/experimental/decompression-api/DecompressionApi.ql:54: Column 5 selects sink.getCall
-    none()
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(DecompressionApiUse).getLocation()
+    or
+    result = sink.(DecompressionApiUse).getCall().getLocation()
   }
 }
 

ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql

@@ -51,14 +51,19 @@ private module PermissivePermissionsConfig implements DataFlow::ConfigSig {
     source.asExpr().getExpr() instanceof PermissivePermissionsExpr
   }
 
-  predicate isSink(DataFlow::Node sink) {
-    exists(FileSystemPermissionModification mod | mod.getAPermissionNode() = sink)
+  additional predicate sinkDef(DataFlow::Node sink, FileSystemPermissionModification mod) {
+    mod.getAPermissionNode() = sink
   }
 
-  predicate observeDiffInformedIncrementalMode() {
-    // TODO(diff-informed): Manually verify if config can be diff-informed.
-    // ql/src/queries/security/cwe-732/WeakFilePermissions.ql:71: Column 5 does not select a source or sink originating from the flow call on line 69
-    none()
+  predicate isSink(DataFlow::Node sink) { sinkDef(sink, _) }
+
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    exists(FileSystemPermissionModification mod |
+      sinkDef(sink, mod) and
+      result = mod.getLocation()
+    )
   }
 }
 
@@ -70,7 +75,8 @@ from
   PermissivePermissionsFlow::PathNode source, PermissivePermissionsFlow::PathNode sink,
   FileSystemPermissionModification mod
 where
-  PermissivePermissionsFlow::flowPath(source, sink) and mod.getAPermissionNode() = sink.getNode()
+  PermissivePermissionsFlow::flowPath(source, sink) and
+  PermissivePermissionsConfig::sinkDef(sink.getNode(), mod)
 select source.getNode(), source, sink,
   "This overly permissive mask used in $@ allows read or write access to others.", mod,
   mod.toString()