Ruby: Recognise more string-valued variables

This increases the sensitivity of our barrier guards.

Author: hmac

ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll

@@ -13,19 +13,18 @@ cached
 private predicate stringConstCompare(CfgNodes::AstCfgNode guard, CfgNode testedNode, boolean branch) {
   exists(CfgNodes::ExprNodes::ComparisonOperationCfgNode c |
     c = guard and
-    exists(CfgNodes::ExprNodes::StringLiteralCfgNode strLitNode |
-      // Only consider strings without any interpolations
-      not strLitNode.getExpr().getComponent(_) instanceof StringInterpolationComponent and
+    exists(ExprCfgNode strNode |
+      strNode.getConstantValue().isStringlikeValue(_) and
       c.getExpr() instanceof EqExpr and
       branch = true
       or
       c.getExpr() instanceof CaseEqExpr and branch = true
       or
       c.getExpr() instanceof NEExpr and branch = false
     |
-      c.getLeftOperand() = strLitNode and c.getRightOperand() = testedNode
+      c.getLeftOperand() = strNode and c.getRightOperand() = testedNode
       or
-      c.getLeftOperand() = testedNode and c.getRightOperand() = strLitNode
+      c.getLeftOperand() = testedNode and c.getRightOperand() = strNode
     )
   )
   or
@@ -193,39 +192,45 @@ private predicate stringConstCaseCompare(
       guard =
         any(CfgNodes::ExprNodes::WhenClauseCfgNode branchNode |
           branchNode = case.getBranch(_) and
-          // For simplicity, consider patterns that contain only string literals or arrays of string literals
+          // For simplicity, consider patterns that contain only string literals, string-valued variables, or arrays of the same.
           forall(ExprCfgNode pattern | pattern = branchNode.getPattern(_) |
+            // foo = "foo"
+            //
+            // when foo
+            // when foo, bar
             // when "foo"
             // when "foo", "bar"
-            pattern instanceof ExprNodes::StringLiteralCfgNode
+            pattern.getConstantValue().isStringlikeValue(_)
             or
             pattern =
               any(CfgNodes::ExprNodes::SplatExprCfgNode splat |
                 // when *["foo", "bar"]
                 forex(ExprCfgNode elem |
                   elem = splat.getOperand().(ExprNodes::ArrayLiteralCfgNode).getAnArgument()
                 |
-                  elem instanceof ExprNodes::StringLiteralCfgNode
+                  elem.getConstantValue().isStringlikeValue(_)
                 )
                 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
+                    elem.getConstantValue().isStringlikeValue(_)
                   )
                 )
               )
           )
         )
       or
+      // foo = "foo"
+      //
+      // in foo
       // in "foo"
-      exists(
-        CfgNodes::ExprNodes::InClauseCfgNode branchNode, ExprNodes::StringLiteralCfgNode pattern
-      |
+      exists(CfgNodes::ExprNodes::InClauseCfgNode branchNode, ExprCfgNode pattern |
         branchNode = case.getBranch(_) and
         pattern = branchNode.getPattern() and
+        pattern.getConstantValue().isStringlikeValue(_) and
         guard = pattern
       )
     )

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected

@@ -1,16 +1,22 @@
 WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:10,3-15)
 failures
-| barrier-guards.rb:288:9:288:19 | # $ guarded | Missing result:guarded= |
 oldStyleBarrierGuards
 | barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
+| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:11:3:15 | "foo" | true |
 | barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:5:10:7 | foo | barrier-guards.rb:9:21:9:23 | foo | true |
 | barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:18:5:18:7 | foo | barrier-guards.rb:15:4:15:6 | foo | false |
+| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:18:5:18:7 | foo | barrier-guards.rb:15:11:15:15 | "foo" | false |
 | barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:24:5:24:7 | foo | barrier-guards.rb:21:8:21:10 | foo | true |
+| barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:24:5:24:7 | foo | barrier-guards.rb:21:15:21:19 | "foo" | true |
 | barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:28:5:28:7 | foo | barrier-guards.rb:27:8:27:10 | foo | false |
+| barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:28:5:28:7 | foo | barrier-guards.rb:27:15:27:19 | "foo" | false |
 | barrier-guards.rb:37:4:37:20 | call to include? | barrier-guards.rb:38:5:38:7 | foo | barrier-guards.rb:37:17:37:19 | foo | true |
 | barrier-guards.rb:43:4:43:15 | ... == ... | barrier-guards.rb:45:9:45:11 | foo | barrier-guards.rb:43:4:43:6 | foo | true |
+| barrier-guards.rb:43:4:43:15 | ... == ... | barrier-guards.rb:45:9:45:11 | foo | barrier-guards.rb:43:11:43:15 | "foo" | true |
 | barrier-guards.rb:70:4:70:21 | call to include? | barrier-guards.rb:71:5:71:7 | foo | barrier-guards.rb:70:18:70:20 | foo | true |
+| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:83:5:83:7 | foo | barrier-guards.rb:82:4:82:18 | call to index | false |
 | barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:83:5:83:7 | foo | barrier-guards.rb:82:15:82:17 | foo | true |
+| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:83:5:83:7 | foo | barrier-guards.rb:82:23:82:25 | nil | false |
 | barrier-guards.rb:207:4:207:15 | ... == ... | barrier-guards.rb:208:5:208:7 | foo | barrier-guards.rb:207:4:207:6 | foo | true |
 | barrier-guards.rb:211:10:211:21 | ... == ... | barrier-guards.rb:212:5:212:7 | foo | barrier-guards.rb:211:10:211:12 | foo | true |
 | barrier-guards.rb:215:16:215:27 | ... == ... | barrier-guards.rb:216:5:216:7 | foo | barrier-guards.rb:215:16:215:18 | foo | true |
@@ -19,9 +25,11 @@ oldStyleBarrierGuards
 | barrier-guards.rb:219:21:219:32 | ... == ... | barrier-guards.rb:220:5:220:7 | foo | barrier-guards.rb:219:21:219:23 | foo | true |
 | barrier-guards.rb:232:6:232:17 | ... == ... | barrier-guards.rb:233:5:233:7 | foo | barrier-guards.rb:232:6:232:8 | foo | true |
 | barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:237:24:237:26 | foo | barrier-guards.rb:237:6:237:8 | foo | true |
-| barrier-guards.rb:268:1:268:12 | ... == ... | barrier-guards.rb:268:17:268:19 | foo | barrier-guards.rb:268:1:268:3 | foo | true |
-| barrier-guards.rb:271:4:271:19 | call to include? | barrier-guards.rb:272:5:272:7 | foo | barrier-guards.rb:271:17:271:19 | foo | true |
-| barrier-guards.rb:277:4:277:20 | call to include? | barrier-guards.rb:278:5:278:7 | foo | barrier-guards.rb:277:18:277:20 | foo | true |
+| barrier-guards.rb:259:4:259:16 | ... == ... | barrier-guards.rb:260:5:260:7 | foo | barrier-guards.rb:259:4:259:6 | foo | true |
+| barrier-guards.rb:264:4:264:16 | ... == ... | barrier-guards.rb:265:5:265:7 | foo | barrier-guards.rb:264:4:264:6 | foo | true |
+| barrier-guards.rb:272:1:272:12 | ... == ... | barrier-guards.rb:272:17:272:19 | foo | barrier-guards.rb:272:1:272:3 | foo | true |
+| barrier-guards.rb:275:4:275:19 | call to include? | barrier-guards.rb:276:5:276:7 | foo | barrier-guards.rb:275:17:275:19 | foo | true |
+| barrier-guards.rb:281:4:281:20 | call to include? | barrier-guards.rb:282:5:282:7 | foo | barrier-guards.rb:281:18:281:20 | foo | true |
 newStyleBarrierGuards
 | barrier-guards.rb:4:5:4:7 | foo |
 | barrier-guards.rb:10:5:10:7 | foo |
@@ -52,9 +60,12 @@ newStyleBarrierGuards
 | barrier-guards.rb:233:5:233:7 | foo |
 | barrier-guards.rb:237:24:237:26 | foo |
 | barrier-guards.rb:244:5:244:7 | foo |
-| barrier-guards.rb:268:17:268:19 | foo |
-| barrier-guards.rb:272:5:272:7 | foo |
-| barrier-guards.rb:278:5:278:7 | foo |
+| barrier-guards.rb:260:5:260:7 | foo |
+| barrier-guards.rb:265:5:265:7 | foo |
+| barrier-guards.rb:272:17:272:19 | foo |
+| barrier-guards.rb:276:5:276:7 | foo |
+| barrier-guards.rb:282:5:282:7 | foo |
+| barrier-guards.rb:292:5:292: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 |
@@ -318,32 +329,35 @@ controls
 | barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:260:5:260:7 | foo | match |
 | barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:264:1:266:3 | if ... | match |
 | barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:265:5:265:7 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:268:1:268:19 | ... && ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:268:17:268:19 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:269:1:269:19 | ... && ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:269:8:269:10 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:271:1:273:3 | if ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:272:5:272:7 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:277:1:279:3 | if ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:278:5:278:7 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:282:1:284:3 | if ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:283:5:283:7 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:286:1:291:3 | case ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:287:1:288:19 | [match] when ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:287:1:288:19 | [no-match] when ... | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:288:5:288:7 | foo | match |
-| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:290:5:290:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:268:1:270:3 | if ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:269:5:269:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:272:1:272:19 | ... && ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:272:17:272:19 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:273:1:273:19 | ... && ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:273:8:273:10 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:275:1:277:3 | if ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:276:5:276:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:281:1:283:3 | if ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:282:5:282:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:286:1:288:3 | if ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:287:5:287:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:290:1:295:3 | case ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:291:1:292:19 | [match] when ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:291:1:292:19 | [no-match] when ... | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:292:5:292:7 | foo | match |
+| barrier-guards.rb:250:4:250:8 | "foo" | barrier-guards.rb:294:5:294:7 | foo | match |
 | barrier-guards.rb:254:4:254:28 | ... == ... | barrier-guards.rb:255:5:255:7 | foo | true |
 | barrier-guards.rb:259:4:259:16 | ... == ... | barrier-guards.rb:260:5:260:7 | foo | true |
 | barrier-guards.rb:264:4:264:16 | ... == ... | barrier-guards.rb:265:5:265:7 | foo | true |
-| barrier-guards.rb:268:1:268:12 | ... == ... | barrier-guards.rb:268:17:268:19 | foo | true |
-| barrier-guards.rb:269:1:269:3 | foo | barrier-guards.rb:269:8:269:10 | foo | true |
-| barrier-guards.rb:271:4:271:19 | call to include? | barrier-guards.rb:272:5:272:7 | foo | true |
-| barrier-guards.rb:277:4:277:20 | call to include? | barrier-guards.rb:278:5:278:7 | foo | true |
-| barrier-guards.rb:282:4:282:20 | call to include? | barrier-guards.rb:283:5:283:7 | foo | true |
-| barrier-guards.rb:287:1:288:19 | [match] when ... | barrier-guards.rb:288:5:288:7 | foo | match |
-| barrier-guards.rb:287:1:288:19 | [no-match] when ... | barrier-guards.rb:290:5:290:7 | foo | no-match |
-| barrier-guards.rb:287:6:287:6 | g | barrier-guards.rb:287:1:288:19 | [match] when ... | match |
-| barrier-guards.rb:287:6:287:6 | g | barrier-guards.rb:287:1:288:19 | [no-match] when ... | no-match |
-| barrier-guards.rb:287:6:287:6 | g | barrier-guards.rb:288:5:288:7 | foo | match |
-| barrier-guards.rb:287:6:287:6 | g | barrier-guards.rb:290:5:290:7 | foo | no-match |
+| barrier-guards.rb:268:4:268:30 | ... == ... | barrier-guards.rb:269:5:269:7 | foo | true |
+| barrier-guards.rb:272:1:272:12 | ... == ... | barrier-guards.rb:272:17:272:19 | foo | true |
+| barrier-guards.rb:273:1:273:3 | foo | barrier-guards.rb:273:8:273:10 | foo | true |
+| barrier-guards.rb:275:4:275:19 | call to include? | barrier-guards.rb:276:5:276:7 | foo | true |
+| barrier-guards.rb:281:4:281:20 | call to include? | barrier-guards.rb:282:5:282:7 | foo | true |
+| barrier-guards.rb:286:4:286:20 | call to include? | barrier-guards.rb:287:5:287:7 | foo | true |
+| barrier-guards.rb:291:1:292:19 | [match] when ... | barrier-guards.rb:292:5:292:7 | foo | match |
+| barrier-guards.rb:291:1:292:19 | [no-match] when ... | barrier-guards.rb:294:5:294:7 | foo | no-match |
+| barrier-guards.rb:291:6:291:6 | g | barrier-guards.rb:291:1:292:19 | [match] when ... | match |
+| barrier-guards.rb:291:6:291:6 | g | barrier-guards.rb:291:1:292:19 | [no-match] when ... | no-match |
+| barrier-guards.rb:291:6:291:6 | g | barrier-guards.rb:292:5:292:7 | foo | match |
+| barrier-guards.rb:291:6:291:6 | g | barrier-guards.rb:294:5:294:7 | foo | no-match |

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb

@@ -257,12 +257,16 @@
 
 F = "foo"
 if foo == "#{F}"
-    foo # $ MISSING: guarded
+    foo # $ guarded
 end
 
 f = "foo"
 if foo == "#{f}"
-    foo # $ MISSING: guarded
+    foo # $ guarded
+end
+
+if foo == "#{f}#{unknown_var}"
+    foo
 end
 
 foo == "foo" && foo # $ guarded