Ruby: Add case string comparison barrier guard

This recognises barriers of the form STRINGS = ["foo", "bar"] case foo when "some string literal" foo when *["other", "strings"] foo when *STRINGS foo end where the reads of `foo` inside each `when` are guarded by the comparison of `foo` with the string literals. We don't yet recognise this construct: case foo when "foo", "bar" foo end This is due to a limitation in the shared barrier guard logic.
2026-04-30 19:26:02 +02:00 · 2022-10-18 11:48:42 +13:00
parent cfbaf5e53b
commit 4bc9096446
9 changed files with 233 additions and 37 deletions
--- a/ruby/ql/lib/change-notes/2022-10-26-case-expression-barrier-guard.md
+++ b/ruby/ql/lib/change-notes/2022-10-26-case-expression-barrier-guard.md
@@ -0,0 +1,4 @@
+---
+ category: minorAnalysis
+---
+ * String literals and arrays of string literals in case expression patterns are now recognised as barrier guards.
--- a/ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll
+++ b/ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll
@@ -234,7 +234,7 @@ module ExprNodes {
    override predicate relevantChild(AstNode n) { none() }
  }

-  /** A control-flow node that wraps an `ArrayLiteral` AST expression. */
+  /** A control-flow node that wraps a `Literal` AST expression. */
  class LiteralCfgNode extends ExprCfgNode {
    override string getAPrimaryQlClass() { result = "LiteralCfgNode" }

@@ -866,6 +866,17 @@ module ExprNodes {
    final override RelationalOperation getExpr() { result = super.getExpr() }
  }

+  /** A control-flow node that wraps a `SplatExpr` AST expression. */
+  class SplatExprCfgNode extends ExprCfgNode {
+    override string getAPrimaryQlClass() { result = "SplatExprCfgNode" }
+
+    SplatExprCfgNode() { e instanceof SplatExpr }
+
+    final override SplatExpr getExpr() { result = super.getExpr() }
+
+    final ExprCfgNode getOperand() { result.getExpr() = e.(SplatExpr).getOperand() }
+  }
+
  /** A control-flow node that wraps an `ElementReference` AST expression. */
  class ElementReferenceCfgNode extends MethodCallCfgNode {
    override string getAPrimaryQlClass() { result = "ElementReferenceCfgNode" }
--- a/ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll
@@ -8,9 +8,9 @@ private import codeql.ruby.dataflow.SSA
 private import codeql.ruby.ast.internal.Constant
 private import codeql.ruby.InclusionTests

-private predicate stringConstCompare(CfgNodes::ExprCfgNode g, CfgNode e, boolean branch) {
+private predicate stringConstCompare(CfgNodes::AstCfgNode guard, CfgNode testedNode, boolean branch) {
  exists(CfgNodes::ExprNodes::ComparisonOperationCfgNode c |
-    c = g and
+    c = guard and
    exists(CfgNodes::ExprNodes::StringLiteralCfgNode strLitNode |
      c.getExpr() instanceof EqExpr and branch = true
      or
@@ -18,11 +18,13 @@ private predicate stringConstCompare(CfgNodes::ExprCfgNode g, CfgNode e, boolean
      or
      c.getExpr() instanceof NEExpr and branch = false
    |
-      c.getLeftOperand() = strLitNode and c.getRightOperand() = e
+      c.getLeftOperand() = strLitNode and c.getRightOperand() = testedNode
      or
-      c.getLeftOperand() = e and c.getRightOperand() = strLitNode
+      c.getLeftOperand() = testedNode and c.getRightOperand() = strLitNode
    )
  )
+  or
+  stringConstCaseCompare(guard, testedNode, branch)
 }

 /**
@@ -72,10 +74,12 @@ deprecated class StringConstCompare extends DataFlow::BarrierGuard,
  }
 }

-private predicate stringConstArrayInclusionCall(CfgNodes::ExprCfgNode g, CfgNode e, boolean branch) {
+private predicate stringConstArrayInclusionCall(
+  CfgNodes::AstCfgNode guard, CfgNode testedNode, boolean branch
+) {
  exists(InclusionTest t |
-    t.asExpr() = g and
-    e = t.getContainedNode().asExpr() and
+    t.asExpr() = guard and
+    testedNode = t.getContainedNode().asExpr() and
    branch = t.getPolarity()
  |
    exists(ExprNodes::ArrayLiteralCfgNode arr |
@@ -132,3 +136,57 @@ deprecated class StringConstArrayInclusionCall extends DataFlow::BarrierGuard,

  override predicate checks(CfgNode expr, boolean branch) { expr = checkedNode and branch = true }
 }
+
+/**
+ * A validation of a value by comparing with a constant string via a `case`
+ * expression. For example:
+ *
+ * ```rb
+ * name = params[:user_name]
+ * case name
+ * when "alice"
+ *   User.find_by("username = #{name}")
+ * when *["bob", "charlie"]
+ *   User.find_by("username = #{name}")
+ * when "dave", "eve" # this is not yet recognised as a barrier guard
+ *   User.find_by("username = #{name}")
+ * end
+ * ```
+ */
+private predicate stringConstCaseCompare(
+  CfgNodes::AstCfgNode guard, CfgNode testedNode, boolean branch
+) {
+  branch = true and
+  exists(CfgNodes::ExprNodes::CaseExprCfgNode case |
+    case.getValue() = testedNode and
+    exists(CfgNodes::ExprNodes::WhenClauseCfgNode branchNode |
+      branchNode = case.getBranch(_) and
+      guard = branchNode.getPattern(_) and
+      // For simplicity, consider patterns that contain only string literals or arrays of string literals
+      forall(ExprCfgNode pattern | pattern = branchNode.getPattern(_) |
+        // when "foo"
+        // when "foo", "bar"
+        pattern instanceof ExprNodes::StringLiteralCfgNode
+        or
+        // array literals behave weirdly in the CFG so we need to drop down to the AST level for this bit
+        // specifically: `SplatExprCfgNode.getOperand()` does not return results for array literals
+        exists(CfgNodes::ExprNodes::SplatExprCfgNode splat | splat = pattern |
+          // when *["foo", "bar"]
+          exists(ArrayLiteral arr |
+            splat.getExpr().getOperand() = arr and
+            forall(Expr elem | elem = arr.getAnElement() | elem instanceof StringLiteral)
+          )
+          or
+          // when *some_var
+          // when *SOME_CONST
+          exists(ExprNodes::ArrayLiteralCfgNode arr |
+            isArrayConstant(splat.getOperand(), arr) and
+            forall(ExprCfgNode elem | elem = arr.getAnArgument() |
+              elem instanceof ExprNodes::StringLiteralCfgNode
+            )
+          )
+        )
+      )
+    )
+  )
+}
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
@@ -434,7 +434,7 @@ class ContentSet extends TContentSet {
 * For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
 * the argument `x`.
 */
-signature predicate guardChecksSig(CfgNodes::ExprCfgNode g, CfgNode e, boolean branch);
+signature predicate guardChecksSig(CfgNodes::AstCfgNode g, CfgNode e, boolean branch);

 /**
 * Provides a set of barrier nodes for a guard that validates an expression.
@@ -444,13 +444,13 @@ signature predicate guardChecksSig(CfgNodes::ExprCfgNode g, CfgNode e, boolean b
 */
 module BarrierGuard<guardChecksSig/3 guardChecks> {
  pragma[nomagic]
-  private predicate guardChecksSsaDef(CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def) {
+  private predicate guardChecksSsaDef(CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def) {
    guardChecks(g, def.getARead(), branch)
  }

  pragma[nomagic]
  private predicate guardControlsSsaDef(
-    CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def, Node n
+    CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def, Node n
  ) {
    def.getARead() = n.asExpr() and
    guardControlsBlock(g, n.asExpr().getBasicBlock(), branch)
@@ -458,7 +458,7 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {

  /** Gets a node that is safely guarded by the given guard check. */
  Node getABarrierNode() {
-    exists(CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def |
+    exists(CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def |
      guardChecksSsaDef(g, branch, def) and
      guardControlsSsaDef(g, branch, def, result)
    )
@@ -488,8 +488,8 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
 }

 /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
-private predicate guardControlsBlock(CfgNodes::ExprCfgNode guard, BasicBlock bb, boolean branch) {
-  exists(ConditionBlock conditionBlock, SuccessorTypes::BooleanSuccessor s |
+private predicate guardControlsBlock(CfgNodes::AstCfgNode guard, BasicBlock bb, boolean branch) {
+  exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s |
    guard = conditionBlock.getLastNode() and
    s.getValue() = branch and
    conditionBlock.controls(bb, s)
--- a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSCustomizations.qll
+++ b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSCustomizations.qll
@@ -127,7 +127,7 @@ module PolynomialReDoS {
    override DataFlow::Node getHighlight() { result = matchNode }
  }

-  private predicate lengthGuard(CfgNodes::ExprCfgNode g, CfgNode node, boolean branch) {
+  private predicate lengthGuard(CfgNodes::AstCfgNode g, CfgNode node, boolean branch) {
    exists(DataFlow::Node input, DataFlow::CallNode length, DataFlow::ExprNode operand |
      length.asExpr().getExpr().(Ast::MethodCall).getMethodName() = "length" and
      length.getReceiver() = input and
--- a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected
+++ b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected
@@ -20,6 +20,13 @@ newStyleBarrierGuards
 | barrier-guards.rb:71:5:71:7 | foo |
 | barrier-guards.rb:83:5:83:7 | foo |
 | barrier-guards.rb:91:5:91:7 | foo |
+| barrier-guards.rb:126:5:126:7 | foo |
+| barrier-guards.rb:133:5:133:7 | foo |
+| barrier-guards.rb:135:5:135:7 | foo |
+| barrier-guards.rb:149:5:149:7 | foo |
+| barrier-guards.rb:154:5:154:7 | foo |
+| barrier-guards.rb:159:5:159:7 | foo |
+| barrier-guards.rb:164:5:164:7 | foo |
 controls
 | barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | true |
 | barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:6:5:6:7 | foo | false |
@@ -67,3 +74,33 @@ controls
 | barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:118:4:118:8 | [true] not ... | false |
 | barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:119:5:119:7 | foo | false |
 | barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:121:5:121:8 | bars | true |
+| barrier-guards.rb:125:6:125:10 | "foo" | barrier-guards.rb:126:5:126:7 | foo | match |
+| barrier-guards.rb:125:6:125:10 | "foo" | barrier-guards.rb:128:5:128:7 | foo | no-match |
+| barrier-guards.rb:132:6:132:10 | "foo" | barrier-guards.rb:133:5:133:7 | foo | match |
+| barrier-guards.rb:132:6:132:10 | "foo" | barrier-guards.rb:134:1:135:7 | when ... | no-match |
+| barrier-guards.rb:132:6:132:10 | "foo" | barrier-guards.rb:135:5:135:7 | foo | no-match |
+| barrier-guards.rb:134:6:134:10 | "bar" | barrier-guards.rb:135:5:135:7 | foo | match |
+| barrier-guards.rb:139:6:139:10 | "foo" | barrier-guards.rb:139:14:139:16 | bar | no-match |
+| barrier-guards.rb:139:6:139:10 | "foo" | barrier-guards.rb:141:1:142:7 | when ... | no-match |
+| barrier-guards.rb:139:6:139:10 | "foo" | barrier-guards.rb:141:14:141:17 | quux | no-match |
+| barrier-guards.rb:139:6:139:10 | "foo" | barrier-guards.rb:142:5:142:7 | foo | no-match |
+| barrier-guards.rb:139:6:139:10 | "foo" | barrier-guards.rb:144:5:144:7 | foo | no-match |
+| barrier-guards.rb:139:13:139:17 | "bar" | barrier-guards.rb:141:1:142:7 | when ... | no-match |
+| barrier-guards.rb:139:13:139:17 | "bar" | barrier-guards.rb:141:14:141:17 | quux | no-match |
+| barrier-guards.rb:139:13:139:17 | "bar" | barrier-guards.rb:142:5:142:7 | foo | no-match |
+| barrier-guards.rb:139:13:139:17 | "bar" | barrier-guards.rb:144:5:144:7 | foo | no-match |
+| barrier-guards.rb:141:6:141:10 | "baz" | barrier-guards.rb:141:14:141:17 | quux | no-match |
+| barrier-guards.rb:141:6:141:10 | "baz" | barrier-guards.rb:144:5:144:7 | foo | no-match |
+| barrier-guards.rb:141:13:141:18 | "quux" | barrier-guards.rb:144:5:144:7 | foo | no-match |
+| barrier-guards.rb:148:6:148:20 | * ... | barrier-guards.rb:149:5:149:7 | foo | match |
+| barrier-guards.rb:153:6:153:17 | * ... | barrier-guards.rb:154:5:154:7 | foo | match |
+| barrier-guards.rb:158:6:158:9 | * ... | barrier-guards.rb:159:5:159:7 | foo | match |
+| barrier-guards.rb:163:6:163:10 | * ... | barrier-guards.rb:164:5:164:7 | foo | match |
+| barrier-guards.rb:168:6:168:16 | * ... | barrier-guards.rb:169:5:169:7 | foo | match |
+| barrier-guards.rb:173:6:173:10 | "foo" | barrier-guards.rb:173:13:173:13 | self | no-match |
+| barrier-guards.rb:180:6:180:15 | * ... | barrier-guards.rb:181:5:181:7 | foo | match |
+| barrier-guards.rb:187:6:187:15 | * ... | barrier-guards.rb:188:5:188:7 | foo | match |
+| barrier-guards.rb:191:4:191:15 | ... == ... | barrier-guards.rb:191:4:191:31 | [false] ... or ... | false |
+| barrier-guards.rb:191:4:191:15 | ... == ... | barrier-guards.rb:191:20:191:22 | foo | false |
+| barrier-guards.rb:191:4:191:31 | [true] ... or ... | barrier-guards.rb:192:5:192:7 | foo | true |
+| barrier-guards.rb:191:20:191:31 | ... == ... | barrier-guards.rb:191:4:191:31 | [false] ... or ... | false |
--- a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb
+++ b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb
@@ -120,3 +120,74 @@ if not x then
 else
    bars
 end
+
+case foo
+when "foo"
+    foo
+else
+    foo
+end
+
+case foo
+when "foo"
+    foo
+when "bar"
+    foo
+end
+
+case foo
+when "foo", "bar" # not recognised
+    foo
+when "baz", "quux" # not recognised
+    foo
+else
+    foo
+end
+
+case foo
+when *["foo", "bar"]
+    foo
+end
+
+case foo
+when *%w[foo bar]
+    foo
+end
+
+case foo
+when *FOO
+    foo
+end
+
+case foo
+when *foos
+    foo
+end
+
+case foo
+when *["foo", x] # not a guard - includes non-constant element `x`
+    foo
+end
+
+case foo
+when "foo", x # not a guard - includes non-constant element `x`
+    foo
+end
+
+foo_and_x = ["foo", x]
+
+case foo
+when *foo_and_x # not a guard - includes non-constant element `x`
+    foo
+end
+
+FOO_AND_X = ["foo", x]
+
+case foo
+when *FOO_AND_X # not a guard - includes non-constant element `x`
+    foo
+end
+
+if foo == "foo" or foo == "bar" # not recognised
+    foo
+end
--- a/ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected
+++ b/ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected
@@ -10,13 +10,15 @@ edges
 | CommandInjection.rb:6:15:6:26 | ...[...] :  | CommandInjection.rb:34:39:34:51 | "grep #{...}" |
 | CommandInjection.rb:46:15:46:20 | call to params :  | CommandInjection.rb:46:15:46:26 | ...[...] :  |
 | CommandInjection.rb:46:15:46:26 | ...[...] :  | CommandInjection.rb:50:24:50:36 | "echo #{...}" |
-| CommandInjection.rb:64:18:64:23 | number :  | CommandInjection.rb:65:14:65:29 | "echo #{...}" |
-| CommandInjection.rb:72:23:72:33 | blah_number :  | CommandInjection.rb:73:14:73:34 | "echo #{...}" |
-| CommandInjection.rb:81:20:81:25 | **args :  | CommandInjection.rb:82:22:82:25 | args :  |
-| CommandInjection.rb:82:22:82:25 | args :  | CommandInjection.rb:82:22:82:37 | ...[...] :  |
-| CommandInjection.rb:82:22:82:37 | ...[...] :  | CommandInjection.rb:82:14:82:39 | "echo #{...}" |
-| CommandInjection.rb:94:16:94:21 | call to params :  | CommandInjection.rb:94:16:94:28 | ...[...] :  |
-| CommandInjection.rb:94:16:94:28 | ...[...] :  | CommandInjection.rb:95:16:95:28 | "cat #{...}" |
+| CommandInjection.rb:54:13:54:18 | call to params :  | CommandInjection.rb:54:13:54:24 | ...[...] :  |
+| CommandInjection.rb:54:13:54:24 | ...[...] :  | CommandInjection.rb:59:14:59:16 | cmd |
+| CommandInjection.rb:73:18:73:23 | number :  | CommandInjection.rb:74:14:74:29 | "echo #{...}" |
+| CommandInjection.rb:81:23:81:33 | blah_number :  | CommandInjection.rb:82:14:82:34 | "echo #{...}" |
+| CommandInjection.rb:90:20:90:25 | **args :  | CommandInjection.rb:91:22:91:25 | args :  |
+| CommandInjection.rb:91:22:91:25 | args :  | CommandInjection.rb:91:22:91:37 | ...[...] :  |
+| CommandInjection.rb:91:22:91:37 | ...[...] :  | CommandInjection.rb:91:14:91:39 | "echo #{...}" |
+| CommandInjection.rb:103:16:103:21 | call to params :  | CommandInjection.rb:103:16:103:28 | ...[...] :  |
+| CommandInjection.rb:103:16:103:28 | ...[...] :  | CommandInjection.rb:104:16:104:28 | "cat #{...}" |
 nodes
 | CommandInjection.rb:6:15:6:20 | call to params :  | semmle.label | call to params :  |
 | CommandInjection.rb:6:15:6:26 | ...[...] :  | semmle.label | ...[...] :  |
@@ -31,17 +33,20 @@ nodes
 | CommandInjection.rb:46:15:46:20 | call to params :  | semmle.label | call to params :  |
 | CommandInjection.rb:46:15:46:26 | ...[...] :  | semmle.label | ...[...] :  |
 | CommandInjection.rb:50:24:50:36 | "echo #{...}" | semmle.label | "echo #{...}" |
-| CommandInjection.rb:64:18:64:23 | number :  | semmle.label | number :  |
-| CommandInjection.rb:65:14:65:29 | "echo #{...}" | semmle.label | "echo #{...}" |
-| CommandInjection.rb:72:23:72:33 | blah_number :  | semmle.label | blah_number :  |
-| CommandInjection.rb:73:14:73:34 | "echo #{...}" | semmle.label | "echo #{...}" |
-| CommandInjection.rb:81:20:81:25 | **args :  | semmle.label | **args :  |
-| CommandInjection.rb:82:14:82:39 | "echo #{...}" | semmle.label | "echo #{...}" |
-| CommandInjection.rb:82:22:82:25 | args :  | semmle.label | args :  |
-| CommandInjection.rb:82:22:82:37 | ...[...] :  | semmle.label | ...[...] :  |
-| CommandInjection.rb:94:16:94:21 | call to params :  | semmle.label | call to params :  |
-| CommandInjection.rb:94:16:94:28 | ...[...] :  | semmle.label | ...[...] :  |
-| CommandInjection.rb:95:16:95:28 | "cat #{...}" | semmle.label | "cat #{...}" |
+| CommandInjection.rb:54:13:54:18 | call to params :  | semmle.label | call to params :  |
+| CommandInjection.rb:54:13:54:24 | ...[...] :  | semmle.label | ...[...] :  |
+| CommandInjection.rb:59:14:59:16 | cmd | semmle.label | cmd |
+| CommandInjection.rb:73:18:73:23 | number :  | semmle.label | number :  |
+| CommandInjection.rb:74:14:74:29 | "echo #{...}" | semmle.label | "echo #{...}" |
+| CommandInjection.rb:81:23:81:33 | blah_number :  | semmle.label | blah_number :  |
+| CommandInjection.rb:82:14:82:34 | "echo #{...}" | semmle.label | "echo #{...}" |
+| CommandInjection.rb:90:20:90:25 | **args :  | semmle.label | **args :  |
+| CommandInjection.rb:91:14:91:39 | "echo #{...}" | semmle.label | "echo #{...}" |
+| CommandInjection.rb:91:22:91:25 | args :  | semmle.label | args :  |
+| CommandInjection.rb:91:22:91:37 | ...[...] :  | semmle.label | ...[...] :  |
+| CommandInjection.rb:103:16:103:21 | call to params :  | semmle.label | call to params :  |
+| CommandInjection.rb:103:16:103:28 | ...[...] :  | semmle.label | ...[...] :  |
+| CommandInjection.rb:104:16:104:28 | "cat #{...}" | semmle.label | "cat #{...}" |
 subpaths
 #select
 | CommandInjection.rb:7:10:7:15 | #{...} | CommandInjection.rb:6:15:6:20 | call to params :  | CommandInjection.rb:7:10:7:15 | #{...} | This command depends on a $@. | CommandInjection.rb:6:15:6:20 | call to params | user-provided value |
@@ -53,7 +58,8 @@ subpaths
 | CommandInjection.rb:33:24:33:36 | "echo #{...}" | CommandInjection.rb:6:15:6:20 | call to params :  | CommandInjection.rb:33:24:33:36 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:6:15:6:20 | call to params | user-provided value |
 | CommandInjection.rb:34:39:34:51 | "grep #{...}" | CommandInjection.rb:6:15:6:20 | call to params :  | CommandInjection.rb:34:39:34:51 | "grep #{...}" | This command depends on a $@. | CommandInjection.rb:6:15:6:20 | call to params | user-provided value |
 | CommandInjection.rb:50:24:50:36 | "echo #{...}" | CommandInjection.rb:46:15:46:20 | call to params :  | CommandInjection.rb:50:24:50:36 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:46:15:46:20 | call to params | user-provided value |
-| CommandInjection.rb:65:14:65:29 | "echo #{...}" | CommandInjection.rb:64:18:64:23 | number :  | CommandInjection.rb:65:14:65:29 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:64:18:64:23 | number | user-provided value |
-| CommandInjection.rb:73:14:73:34 | "echo #{...}" | CommandInjection.rb:72:23:72:33 | blah_number :  | CommandInjection.rb:73:14:73:34 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:72:23:72:33 | blah_number | user-provided value |
-| CommandInjection.rb:82:14:82:39 | "echo #{...}" | CommandInjection.rb:81:20:81:25 | **args :  | CommandInjection.rb:82:14:82:39 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:81:20:81:25 | **args | user-provided value |
-| CommandInjection.rb:95:16:95:28 | "cat #{...}" | CommandInjection.rb:94:16:94:21 | call to params :  | CommandInjection.rb:95:16:95:28 | "cat #{...}" | This command depends on a $@. | CommandInjection.rb:94:16:94:21 | call to params | user-provided value |
+| CommandInjection.rb:59:14:59:16 | cmd | CommandInjection.rb:54:13:54:18 | call to params :  | CommandInjection.rb:59:14:59:16 | cmd | This command depends on a $@. | CommandInjection.rb:54:13:54:18 | call to params | user-provided value |
+| CommandInjection.rb:74:14:74:29 | "echo #{...}" | CommandInjection.rb:73:18:73:23 | number :  | CommandInjection.rb:74:14:74:29 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:73:18:73:23 | number | user-provided value |
+| CommandInjection.rb:82:14:82:34 | "echo #{...}" | CommandInjection.rb:81:23:81:33 | blah_number :  | CommandInjection.rb:82:14:82:34 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:81:23:81:33 | blah_number | user-provided value |
+| CommandInjection.rb:91:14:91:39 | "echo #{...}" | CommandInjection.rb:90:20:90:25 | **args :  | CommandInjection.rb:91:14:91:39 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:90:20:90:25 | **args | user-provided value |
+| CommandInjection.rb:104:16:104:28 | "cat #{...}" | CommandInjection.rb:103:16:103:21 | call to params :  | CommandInjection.rb:104:16:104:28 | "cat #{...}" | This command depends on a $@. | CommandInjection.rb:103:16:103:21 | call to params | user-provided value |
--- a/ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.rb
+++ b/ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.rb
@@ -49,6 +49,15 @@ EOF
        end
        Open3.capture2("echo #{cmd}")
    end
+
+    def update
+      cmd = params[:key]
+      case cmd
+      when "foo"
+        system(cmd)
+      end
+      system(cmd)
+    end
 end

 module Types