Merge pull request #15467 from hmac/hmac-and-barrier

Ruby: Block flow from LHS of && expressions

Author: hmac

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -220,8 +220,10 @@ module LocalFlow {
     or
     nodeTo.asExpr() =
       any(CfgNodes::ExprNodes::BinaryOperationCfgNode op |
-        op.getExpr() instanceof BinaryLogicalOperation and
+        op.getExpr() instanceof LogicalOrExpr and
         nodeFrom.asExpr() = op.getAnOperand()
+        or
+        op.getExpr() instanceof LogicalAndExpr and nodeFrom.asExpr() = op.getRightOperand()
       )
     or
     nodeTo.(ParameterNodeImpl).getParameter() =

ruby/ql/test/library-tests/dataflow/local/DataflowStep.expected

@@ -2590,7 +2590,6 @@
 | local_dataflow.rb:96:3:96:9 | self | local_dataflow.rb:98:7:98:15 | self |
 | local_dataflow.rb:98:3:98:3 | a | local_dataflow.rb:99:8:99:8 | a |
 | local_dataflow.rb:98:7:98:15 | [post] self | local_dataflow.rb:98:20:98:28 | self |
-| local_dataflow.rb:98:7:98:15 | call to source | local_dataflow.rb:98:7:98:28 | ... && ... |
 | local_dataflow.rb:98:7:98:15 | self | local_dataflow.rb:98:20:98:28 | self |
 | local_dataflow.rb:98:7:98:28 | ... && ... | local_dataflow.rb:98:3:98:3 | a |
 | local_dataflow.rb:98:7:98:28 | ... && ... | local_dataflow.rb:98:3:98:28 | ... = ... |
@@ -2602,7 +2601,6 @@
 | local_dataflow.rb:100:7:100:31 | ( ... ) | local_dataflow.rb:100:3:100:3 | b |
 | local_dataflow.rb:100:7:100:31 | ( ... ) | local_dataflow.rb:100:3:100:31 | ... = ... |
 | local_dataflow.rb:100:8:100:16 | [post] self | local_dataflow.rb:100:22:100:30 | self |
-| local_dataflow.rb:100:8:100:16 | call to source | local_dataflow.rb:100:8:100:30 | ... and ... |
 | local_dataflow.rb:100:8:100:16 | self | local_dataflow.rb:100:22:100:30 | self |
 | local_dataflow.rb:100:8:100:30 | ... and ... | local_dataflow.rb:100:7:100:31 | ( ... ) |
 | local_dataflow.rb:100:8:100:30 | SSA phi read(self) | local_dataflow.rb:101:3:101:9 | self |
@@ -2627,7 +2625,6 @@
 | local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:106:3:106:3 | b |
 | local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:106:3:106:15 | ... = ... |
 | local_dataflow.rb:106:7:106:15 | self | local_dataflow.rb:107:9:107:17 | self |
-| local_dataflow.rb:107:3:107:3 | b | local_dataflow.rb:107:5:107:7 | ... && ... |
 | local_dataflow.rb:107:3:107:3 | b | local_dataflow.rb:108:8:108:8 | b |
 | local_dataflow.rb:107:5:107:7 | ... && ... | local_dataflow.rb:107:3:107:3 | b |
 | local_dataflow.rb:107:5:107:7 | ... && ... | local_dataflow.rb:107:3:107:17 | ... = ... |
@@ -2704,8 +2701,6 @@
 | local_dataflow.rb:137:7:138:9 | SSA phi read(x) | local_dataflow.rb:133:5:139:7 | SSA phi read(x) |
 | local_dataflow.rb:137:7:138:9 | if ... | local_dataflow.rb:135:5:138:9 | else ... |
 | local_dataflow.rb:137:10:137:15 | [post] self | local_dataflow.rb:137:21:137:26 | self |
-| local_dataflow.rb:137:10:137:15 | call to use | local_dataflow.rb:137:10:137:26 | [false] ... && ... |
-| local_dataflow.rb:137:10:137:15 | call to use | local_dataflow.rb:137:10:137:26 | [true] ... && ... |
 | local_dataflow.rb:137:10:137:15 | self | local_dataflow.rb:137:21:137:26 | self |
 | local_dataflow.rb:137:10:137:26 | SSA phi read(self) | local_dataflow.rb:137:7:138:9 | SSA phi read(self) |
 | local_dataflow.rb:137:10:137:26 | SSA phi read(x) | local_dataflow.rb:137:7:138:9 | SSA phi read(x) |
@@ -2725,8 +2720,6 @@
 | local_dataflow.rb:141:19:141:37 | [false] ( ... ) | local_dataflow.rb:141:8:141:37 | [false] ... \|\| ... |
 | local_dataflow.rb:141:19:141:37 | [true] ( ... ) | local_dataflow.rb:141:8:141:37 | [true] ... \|\| ... |
 | local_dataflow.rb:141:20:141:25 | [post] self | local_dataflow.rb:141:31:141:36 | self |
-| local_dataflow.rb:141:20:141:25 | call to use | local_dataflow.rb:141:20:141:36 | [false] ... && ... |
-| local_dataflow.rb:141:20:141:25 | call to use | local_dataflow.rb:141:20:141:36 | [true] ... && ... |
 | local_dataflow.rb:141:20:141:25 | self | local_dataflow.rb:141:31:141:36 | self |
 | local_dataflow.rb:141:20:141:36 | SSA phi read(self) | local_dataflow.rb:143:11:143:16 | self |
 | local_dataflow.rb:141:20:141:36 | SSA phi read(x) | local_dataflow.rb:143:15:143:15 | x |

ruby/ql/test/library-tests/dataflow/local/InlineFlowTest.expected

@@ -25,17 +25,13 @@ edges
 | local_dataflow.rb:95:8:95:16 | call to source | local_dataflow.rb:95:3:95:3 | b |
 | local_dataflow.rb:95:21:95:29 | call to source | local_dataflow.rb:95:3:95:3 | b |
 | local_dataflow.rb:98:3:98:3 | a | local_dataflow.rb:99:8:99:8 | a |
-| local_dataflow.rb:98:7:98:15 | call to source | local_dataflow.rb:98:3:98:3 | a |
 | local_dataflow.rb:98:20:98:28 | call to source | local_dataflow.rb:98:3:98:3 | a |
 | local_dataflow.rb:100:3:100:3 | b | local_dataflow.rb:101:8:101:8 | b |
-| local_dataflow.rb:100:8:100:16 | call to source | local_dataflow.rb:100:3:100:3 | b |
 | local_dataflow.rb:100:22:100:30 | call to source | local_dataflow.rb:100:3:100:3 | b |
 | local_dataflow.rb:103:3:103:3 | a | local_dataflow.rb:104:3:104:3 | a |
 | local_dataflow.rb:103:7:103:15 | call to source | local_dataflow.rb:103:3:103:3 | a |
 | local_dataflow.rb:104:3:104:3 | a | local_dataflow.rb:105:8:105:8 | a |
 | local_dataflow.rb:104:9:104:17 | call to source | local_dataflow.rb:104:3:104:3 | a |
-| local_dataflow.rb:106:3:106:3 | b | local_dataflow.rb:107:3:107:3 | b |
-| local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:106:3:106:3 | b |
 | local_dataflow.rb:107:3:107:3 | b | local_dataflow.rb:108:8:108:8 | b |
 | local_dataflow.rb:107:9:107:17 | call to source | local_dataflow.rb:107:3:107:3 | b |
 | local_dataflow.rb:112:8:112:16 | call to source | local_dataflow.rb:112:8:112:20 | call to dup |
@@ -78,20 +74,16 @@ nodes
 | local_dataflow.rb:95:21:95:29 | call to source | semmle.label | call to source |
 | local_dataflow.rb:96:8:96:8 | b | semmle.label | b |
 | local_dataflow.rb:98:3:98:3 | a | semmle.label | a |
-| local_dataflow.rb:98:7:98:15 | call to source | semmle.label | call to source |
 | local_dataflow.rb:98:20:98:28 | call to source | semmle.label | call to source |
 | local_dataflow.rb:99:8:99:8 | a | semmle.label | a |
 | local_dataflow.rb:100:3:100:3 | b | semmle.label | b |
-| local_dataflow.rb:100:8:100:16 | call to source | semmle.label | call to source |
 | local_dataflow.rb:100:22:100:30 | call to source | semmle.label | call to source |
 | local_dataflow.rb:101:8:101:8 | b | semmle.label | b |
 | local_dataflow.rb:103:3:103:3 | a | semmle.label | a |
 | local_dataflow.rb:103:7:103:15 | call to source | semmle.label | call to source |
 | local_dataflow.rb:104:3:104:3 | a | semmle.label | a |
 | local_dataflow.rb:104:9:104:17 | call to source | semmle.label | call to source |
 | local_dataflow.rb:105:8:105:8 | a | semmle.label | a |
-| local_dataflow.rb:106:3:106:3 | b | semmle.label | b |
-| local_dataflow.rb:106:7:106:15 | call to source | semmle.label | call to source |
 | local_dataflow.rb:107:3:107:3 | b | semmle.label | b |
 | local_dataflow.rb:107:9:107:17 | call to source | semmle.label | call to source |
 | local_dataflow.rb:108:8:108:8 | b | semmle.label | b |
@@ -127,13 +119,10 @@ subpaths
 | local_dataflow.rb:94:8:94:8 | a | local_dataflow.rb:93:20:93:28 | call to source | local_dataflow.rb:94:8:94:8 | a | $@ | local_dataflow.rb:93:20:93:28 | call to source | call to source |
 | local_dataflow.rb:96:8:96:8 | b | local_dataflow.rb:95:8:95:16 | call to source | local_dataflow.rb:96:8:96:8 | b | $@ | local_dataflow.rb:95:8:95:16 | call to source | call to source |
 | local_dataflow.rb:96:8:96:8 | b | local_dataflow.rb:95:21:95:29 | call to source | local_dataflow.rb:96:8:96:8 | b | $@ | local_dataflow.rb:95:21:95:29 | call to source | call to source |
-| local_dataflow.rb:99:8:99:8 | a | local_dataflow.rb:98:7:98:15 | call to source | local_dataflow.rb:99:8:99:8 | a | $@ | local_dataflow.rb:98:7:98:15 | call to source | call to source |
 | local_dataflow.rb:99:8:99:8 | a | local_dataflow.rb:98:20:98:28 | call to source | local_dataflow.rb:99:8:99:8 | a | $@ | local_dataflow.rb:98:20:98:28 | call to source | call to source |
-| local_dataflow.rb:101:8:101:8 | b | local_dataflow.rb:100:8:100:16 | call to source | local_dataflow.rb:101:8:101:8 | b | $@ | local_dataflow.rb:100:8:100:16 | call to source | call to source |
 | local_dataflow.rb:101:8:101:8 | b | local_dataflow.rb:100:22:100:30 | call to source | local_dataflow.rb:101:8:101:8 | b | $@ | local_dataflow.rb:100:22:100:30 | call to source | call to source |
 | local_dataflow.rb:105:8:105:8 | a | local_dataflow.rb:103:7:103:15 | call to source | local_dataflow.rb:105:8:105:8 | a | $@ | local_dataflow.rb:103:7:103:15 | call to source | call to source |
 | local_dataflow.rb:105:8:105:8 | a | local_dataflow.rb:104:9:104:17 | call to source | local_dataflow.rb:105:8:105:8 | a | $@ | local_dataflow.rb:104:9:104:17 | call to source | call to source |
-| local_dataflow.rb:108:8:108:8 | b | local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:108:8:108:8 | b | $@ | local_dataflow.rb:106:7:106:15 | call to source | call to source |
 | local_dataflow.rb:108:8:108:8 | b | local_dataflow.rb:107:9:107:17 | call to source | local_dataflow.rb:108:8:108:8 | b | $@ | local_dataflow.rb:107:9:107:17 | call to source | call to source |
 | local_dataflow.rb:112:8:112:20 | call to dup | local_dataflow.rb:112:8:112:16 | call to source | local_dataflow.rb:112:8:112:20 | call to dup | $@ | local_dataflow.rb:112:8:112:16 | call to source | call to source |
 | local_dataflow.rb:113:8:113:24 | call to dup | local_dataflow.rb:113:8:113:16 | call to source | local_dataflow.rb:113:8:113:24 | call to dup | $@ | local_dataflow.rb:113:8:113:16 | call to source | call to source |

ruby/ql/test/library-tests/dataflow/local/TaintStep.expected

@@ -3072,7 +3072,6 @@
 | local_dataflow.rb:96:3:96:9 | self | local_dataflow.rb:98:7:98:15 | self |
 | local_dataflow.rb:98:3:98:3 | a | local_dataflow.rb:99:8:99:8 | a |
 | local_dataflow.rb:98:7:98:15 | [post] self | local_dataflow.rb:98:20:98:28 | self |
-| local_dataflow.rb:98:7:98:15 | call to source | local_dataflow.rb:98:7:98:28 | ... && ... |
 | local_dataflow.rb:98:7:98:15 | self | local_dataflow.rb:98:20:98:28 | self |
 | local_dataflow.rb:98:7:98:28 | ... && ... | local_dataflow.rb:98:3:98:3 | a |
 | local_dataflow.rb:98:7:98:28 | ... && ... | local_dataflow.rb:98:3:98:28 | ... = ... |
@@ -3084,7 +3083,6 @@
 | local_dataflow.rb:100:7:100:31 | ( ... ) | local_dataflow.rb:100:3:100:3 | b |
 | local_dataflow.rb:100:7:100:31 | ( ... ) | local_dataflow.rb:100:3:100:31 | ... = ... |
 | local_dataflow.rb:100:8:100:16 | [post] self | local_dataflow.rb:100:22:100:30 | self |
-| local_dataflow.rb:100:8:100:16 | call to source | local_dataflow.rb:100:8:100:30 | ... and ... |
 | local_dataflow.rb:100:8:100:16 | self | local_dataflow.rb:100:22:100:30 | self |
 | local_dataflow.rb:100:8:100:30 | ... and ... | local_dataflow.rb:100:7:100:31 | ( ... ) |
 | local_dataflow.rb:100:8:100:30 | SSA phi read(self) | local_dataflow.rb:101:3:101:9 | self |
@@ -3109,7 +3107,6 @@
 | local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:106:3:106:3 | b |
 | local_dataflow.rb:106:7:106:15 | call to source | local_dataflow.rb:106:3:106:15 | ... = ... |
 | local_dataflow.rb:106:7:106:15 | self | local_dataflow.rb:107:9:107:17 | self |
-| local_dataflow.rb:107:3:107:3 | b | local_dataflow.rb:107:5:107:7 | ... && ... |
 | local_dataflow.rb:107:3:107:3 | b | local_dataflow.rb:108:8:108:8 | b |
 | local_dataflow.rb:107:5:107:7 | ... && ... | local_dataflow.rb:107:3:107:3 | b |
 | local_dataflow.rb:107:5:107:7 | ... && ... | local_dataflow.rb:107:3:107:17 | ... = ... |
@@ -3189,8 +3186,6 @@
 | local_dataflow.rb:137:7:138:9 | SSA phi read(x) | local_dataflow.rb:133:5:139:7 | SSA phi read(x) |
 | local_dataflow.rb:137:7:138:9 | if ... | local_dataflow.rb:135:5:138:9 | else ... |
 | local_dataflow.rb:137:10:137:15 | [post] self | local_dataflow.rb:137:21:137:26 | self |
-| local_dataflow.rb:137:10:137:15 | call to use | local_dataflow.rb:137:10:137:26 | [false] ... && ... |
-| local_dataflow.rb:137:10:137:15 | call to use | local_dataflow.rb:137:10:137:26 | [true] ... && ... |
 | local_dataflow.rb:137:10:137:15 | self | local_dataflow.rb:137:21:137:26 | self |
 | local_dataflow.rb:137:10:137:26 | SSA phi read(self) | local_dataflow.rb:137:7:138:9 | SSA phi read(self) |
 | local_dataflow.rb:137:10:137:26 | SSA phi read(x) | local_dataflow.rb:137:7:138:9 | SSA phi read(x) |
@@ -3214,8 +3209,6 @@
 | local_dataflow.rb:141:19:141:37 | [false] ( ... ) | local_dataflow.rb:141:8:141:37 | [false] ... \|\| ... |
 | local_dataflow.rb:141:19:141:37 | [true] ( ... ) | local_dataflow.rb:141:8:141:37 | [true] ... \|\| ... |
 | local_dataflow.rb:141:20:141:25 | [post] self | local_dataflow.rb:141:31:141:36 | self |
-| local_dataflow.rb:141:20:141:25 | call to use | local_dataflow.rb:141:20:141:36 | [false] ... && ... |
-| local_dataflow.rb:141:20:141:25 | call to use | local_dataflow.rb:141:20:141:36 | [true] ... && ... |
 | local_dataflow.rb:141:20:141:25 | self | local_dataflow.rb:141:31:141:36 | self |
 | local_dataflow.rb:141:20:141:36 | SSA phi read(self) | local_dataflow.rb:143:11:143:16 | self |
 | local_dataflow.rb:141:20:141:36 | SSA phi read(x) | local_dataflow.rb:143:15:143:15 | x |

ruby/ql/test/library-tests/dataflow/local/local_dataflow.rb

@@ -96,16 +96,16 @@ def and_or
   sink(b) # $ hasValueFlow=1 hasValueFlow=2
 
   a = source(1) && source(2)
-  sink(a) # $ hasValueFlow=1 hasValueFlow=2
+  sink(a) # $ hasValueFlow=2
   b = (source(1) and source(2))
-  sink(b) # $ hasValueFlow=1 hasValueFlow=2
+  sink(b) # $ hasValueFlow=2
 
   a = source(5)
   a ||= source(6)
   sink(a) # $ hasValueFlow=5 hasValueFlow=6
   b = source(7)
   b &&= source(8)
-  sink(b) # $ hasValueFlow=7 hasValueFlow=8
+  sink(b) # $ hasValueFlow=8
 end
 
 def object_dup