Merge pull request #18572 from asgerf/rb/diff-informed2

Ruby: fix and improve diff-informed queries
github · Jan 24, 2025 · a54e732 · a54e732
2 parents 1b7977b + fcb8cac
commit a54e732
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 3 deletions.
diff --git a/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll b/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll
@@ -19,6 +19,10 @@ private module Config implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.getLocation() or result = sink.(Sink).getAction().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll b/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll
@@ -22,6 +22,12 @@ private module InsecureDownloadConfig implements DataFlow::StateConfigSig {
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getDownloadCall().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll
@@ -26,6 +26,12 @@ private module UnsafeCodeConstructionConfig implements DataFlow::ConfigSig {
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getCodeSink().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll
@@ -23,6 +23,12 @@ private module UnsafeHtmlConstructionConfig implements DataFlow::ConfigSig {
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getXssSink().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll
@@ -28,6 +28,14 @@ private module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigS
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
   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/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll
@@ -19,6 +19,14 @@ private module MissingFullAnchorConfig implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getCallNode().getLocation()
+    or
+    result = sink.(Sink).getRegex().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll
@@ -20,6 +20,14 @@ private module PolynomialReDoSConfig implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
+    result = sink.(Sink).getHighlight().getLocation()
+    or
+    result = sink.(Sink).getRegExp().getLocation()
+  }
 }
 
 /**

diff --git a/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql b/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql
@@ -41,6 +41,12 @@ private module DecompressionApiConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse }
 
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(DecompressionApiUse).getLocation()
+    or
+    result = sink.(DecompressionApiUse).getCall().getLocation()
+  }
 }
 
 private module DecompressionApiFlow = TaintTracking::Global<DecompressionApiConfig>;

diff --git a/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql b/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql
@@ -51,11 +51,20 @@ 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 isSink(DataFlow::Node sink) { sinkDef(sink, _) }
+
   predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    exists(FileSystemPermissionModification mod |
+      sinkDef(sink, mod) and
+      result = mod.getLocation()
+    )
+  }
 }
 
 private module PermissivePermissionsFlow = DataFlow::Global<PermissivePermissionsConfig>;
@@ -66,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()