Ruby: Share more code with JS

Author: hmac

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

@@ -157,3 +157,46 @@ private predicate isCommonWordMatcher(RegExpTerm t) {
         .getValue() = "w"
   )
 }
+
+/**
+ * Holds if `replace` has a pattern argument containing a regular expression
+ * `dangerous` which matches a dangerous string beginning with `prefix`, in an
+ * attempt to avoid a vulnerability of kind `kind`.
+ */
+predicate isResult(
+  StringSubstitutionCall replace, EmptyReplaceRegExpTerm dangerous, string prefix, string kind
+) {
+  exists(EmptyReplaceRegExpTerm regexp |
+    replace = regexp.getCall() and
+    dangerous.getRootTerm() = regexp and
+    // skip leading optional elements
+    not dangerous.isNullable() and
+    // only warn about the longest match
+    prefix = max(string m | matchesDangerousPrefix(dangerous, m, kind) | m order by m.length(), m) and
+    // only warn once per kind
+    not exists(EmptyReplaceRegExpTerm other |
+      other = dangerous.getAChild+() or other = dangerous.getPredecessor+()
+    |
+      matchesDangerousPrefix(other, _, kind) and
+      not other.isNullable()
+    ) and
+    // avoid anchored terms
+    not exists(RegExpAnchor a | regexp = a.getRootTerm()) and
+    // Don't flag replace operations that are called repeatedly in a loop, as they can actually work correctly.
+    not replace.flowsTo(replace.getReceiver+())
+  )
+}
+
+/**
+ * Holds if `replace` has a pattern argument containing a regular expression
+ * `dangerous` which matches a dangerous string beginning with `prefix`. `msg`
+ * is the alert we report.
+ */
+query predicate problems(
+  StringSubstitutionCall replace, string msg, EmptyReplaceRegExpTerm dangerous, string prefix
+) {
+  exists(string kind |
+    isResult(replace, dangerous, prefix, kind) and
+    msg = "This string may still contain $@, which may cause a " + kind + " vulnerability."
+  )
+}

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

@@ -2,7 +2,7 @@
  * Provides language-specific predicates for reasoning about improper multi-character sanitization.
  */
 
-private import codeql.ruby.frameworks.core.String
+import codeql.ruby.frameworks.core.String
 import codeql.ruby.regexp.RegExpTreeView
 import codeql.ruby.security.performance.ReDoSUtil as ReDoSUtil
 

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

@@ -13,11 +13,4 @@
  *       external/cwe/cwe-116
  */
 
-import codeql.ruby.DataFlow
-import codeql.ruby.security.IncompleteMultiCharacterSanitizationQuery as Query
-import codeql.ruby.regexp.RegExpTreeView
-
-from DataFlow::Node replace, RegExpTerm dangerous, string prefix, string kind
-where Query::problems(replace, dangerous, prefix, kind)
-select replace, "This string may still contain $@, which may cause a " + kind + " vulnerability.",
-  dangerous, prefix
+import codeql.ruby.security.IncompleteMultiCharacterSanitization

ruby/ql/src/queries/security/cwe-116/IncompleteMultiCharacterSanitization.ql

@@ -5,7 +5,7 @@
 import ruby
 import codeql.ruby.regexp.RegExpTreeView as RETV
 import codeql.ruby.DataFlow
-import codeql.ruby.security.IncompleteMultiCharacterSanitizationQuery as Query
+import codeql.ruby.security.IncompleteMultiCharacterSanitization as Query
 import TestUtilities.InlineExpectationsTest
 
 class Test extends InlineExpectationsTest {
@@ -25,7 +25,7 @@ predicate hasResult(Location location, string element, string value) {
     element = replace.toString() and
     value = shortKind(kind)
   |
-    Query::problems(replace, dangerous, prefix, kind)
+    Query::isResult(replace, dangerous, prefix, kind)
   )
 }