add flow-state support to sanitizers in code-execution, and use that to refactor the string-concatenation-sanitizer

Author: erik-krogh

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

@@ -39,7 +39,13 @@ module CodeInjection {
   /**
    * A sanitizer for "Code injection" vulnerabilities.
    */
-  abstract class Sanitizer extends DataFlow::Node { }
+  abstract class Sanitizer extends DataFlow::Node {
+    /**
+     * Gets a flow state for which this is a sanitizer.
+     * Sanitizes all states if the result is empty.
+     */
+    DataFlow::FlowState getAFlowState() { none() }
+  }
 
   /**
    * DEPRECATED: Use `Sanitizer` instead.
@@ -68,4 +74,24 @@ module CodeInjection {
       else result = FlowState::full() // If it "just" loads something, then it's only vulnerable if the attacker controls the entire string.
     }
   }
+
+  private import codeql.ruby.AST as Ast
+
+  /**
+   * A string-concatenation that sanitizes the `full()` state.
+   */
+  class StringConcatenationSanitizer extends Sanitizer {
+    StringConcatenationSanitizer() {
+      // string concatenations sanitize the `full` state, as an attacker no longer controls the entire string
+      exists(Ast::AstNode str |
+        str instanceof Ast::StringLiteral
+        or
+        str instanceof Ast::AddExpr
+      |
+        this.asExpr().getExpr() = str
+      )
+    }
+
+    override DataFlow::FlowState getAFlowState() { result = FlowState::full() }
+  }
 }

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

@@ -9,7 +9,6 @@ import codeql.ruby.DataFlow
 import codeql.ruby.TaintTracking
 import CodeInjectionCustomizations::CodeInjection
 import codeql.ruby.dataflow.BarrierGuards
-private import codeql.ruby.AST as Ast
 
 /**
  * A taint-tracking configuration for detecting "Code injection" vulnerabilities.
@@ -26,21 +25,15 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
-    node instanceof Sanitizer or
-    node instanceof StringConstCompareBarrier or
+    node instanceof Sanitizer and not exists(node.(Sanitizer).getAFlowState())
+    or
+    node instanceof StringConstCompareBarrier
+    or
     node instanceof StringConstArrayInclusionCallBarrier
   }
 
   override predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) {
-    // string concatenations sanitize the `full` state, as an attacker no longer controls the entire string
-    exists(Ast::AstNode str |
-      str instanceof Ast::StringLiteral
-      or
-      str instanceof Ast::AddExpr
-    |
-      node.asExpr().getExpr() = str and
-      state = FlowState::full()
-    )
+    node.(Sanitizer).getAFlowState() = state
   }
 
   deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {