From 07910b09d0c61a8b3e5b55c8119aa4af173c409d Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Sat, 23 Sep 2023 14:10:15 +0200 Subject: [PATCH 1/2] Ruby: Add more callback flow tests --- .../dataflow/global/Flow.expected | 40 +++++++++++++++++++ .../global/TypeTrackingInlineTest.expected | 2 + .../dataflow/global/callbacks.rb | 29 ++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 ruby/ql/test/library-tests/dataflow/global/callbacks.rb diff --git a/ruby/ql/test/library-tests/dataflow/global/Flow.expected b/ruby/ql/test/library-tests/dataflow/global/Flow.expected index 2b3159fce5bf..2272665cba3a 100644 --- a/ruby/ql/test/library-tests/dataflow/global/Flow.expected +++ b/ruby/ql/test/library-tests/dataflow/global/Flow.expected @@ -5,6 +5,22 @@ edges | blocks.rb:18:11:18:11 | x | blocks.rb:24:18:24:18 | x | provenance | | | blocks.rb:24:3:24:11 | call to source | blocks.rb:17:10:17:10 | x | provenance | | | blocks.rb:24:18:24:18 | x | blocks.rb:25:8:25:8 | x | provenance | | +| callbacks.rb:9:15:9:15 | x | callbacks.rb:10:12:10:12 | x | provenance | | +| callbacks.rb:10:12:10:12 | x | callbacks.rb:17:15:17:15 | x | provenance | | +| callbacks.rb:10:12:10:12 | x | callbacks.rb:18:15:18:15 | x | provenance | | +| callbacks.rb:13:20:13:20 | x | callbacks.rb:14:14:14:14 | x | provenance | | +| callbacks.rb:14:14:14:14 | x | callbacks.rb:9:15:9:15 | x | provenance | | +| callbacks.rb:17:15:17:15 | x | callbacks.rb:17:25:17:25 | x | provenance | | +| callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:13:20:13:20 | x | provenance | | +| callbacks.rb:18:15:18:15 | x | callbacks.rb:18:25:18:25 | x | provenance | | +| callbacks.rb:20:17:20:17 | x | callbacks.rb:21:11:21:11 | x | provenance | | +| callbacks.rb:21:11:21:11 | x | callbacks.rb:28:31:28:31 | x | provenance | | +| callbacks.rb:21:11:21:11 | x | callbacks.rb:29:29:29:29 | x | provenance | | +| callbacks.rb:24:23:24:23 | x | callbacks.rb:25:17:25:17 | x | provenance | | +| callbacks.rb:25:17:25:17 | x | callbacks.rb:20:17:20:17 | x | provenance | | +| callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:24:23:24:23 | x | provenance | | +| callbacks.rb:28:31:28:31 | x | callbacks.rb:28:39:28:39 | x | provenance | | +| callbacks.rb:29:29:29:29 | x | callbacks.rb:29:37:29:37 | x | provenance | | | captured_variables.rb:9:24:9:24 | x | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | provenance | | | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | captured_variables.rb:10:20:10:20 | x | provenance | | | captured_variables.rb:13:20:13:29 | call to taint | captured_variables.rb:9:24:9:24 | x | provenance | | @@ -272,6 +288,24 @@ nodes | blocks.rb:24:3:24:11 | call to source | semmle.label | call to source | | blocks.rb:24:18:24:18 | x | semmle.label | x | | blocks.rb:25:8:25:8 | x | semmle.label | x | +| callbacks.rb:9:15:9:15 | x | semmle.label | x | +| callbacks.rb:10:12:10:12 | x | semmle.label | x | +| callbacks.rb:13:20:13:20 | x | semmle.label | x | +| callbacks.rb:14:14:14:14 | x | semmle.label | x | +| callbacks.rb:17:15:17:15 | x | semmle.label | x | +| callbacks.rb:17:25:17:25 | x | semmle.label | x | +| callbacks.rb:17:31:17:38 | call to taint | semmle.label | call to taint | +| callbacks.rb:18:15:18:15 | x | semmle.label | x | +| callbacks.rb:18:25:18:25 | x | semmle.label | x | +| callbacks.rb:20:17:20:17 | x | semmle.label | x | +| callbacks.rb:21:11:21:11 | x | semmle.label | x | +| callbacks.rb:24:23:24:23 | x | semmle.label | x | +| callbacks.rb:25:17:25:17 | x | semmle.label | x | +| callbacks.rb:28:18:28:25 | call to taint | semmle.label | call to taint | +| callbacks.rb:28:31:28:31 | x | semmle.label | x | +| callbacks.rb:28:39:28:39 | x | semmle.label | x | +| callbacks.rb:29:29:29:29 | x | semmle.label | x | +| callbacks.rb:29:37:29:37 | x | semmle.label | x | | captured_variables.rb:9:24:9:24 | x | semmle.label | x | | captured_variables.rb:10:20:10:20 | x | semmle.label | x | | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | semmle.label | fn : [lambda] [captured x] | @@ -582,9 +616,15 @@ subpaths | instance_variables.rb:120:6:120:10 | foo16 : Foo [@field] | captured_variables.rb:60:5:62:7 | self in get_field : Foo [@field] | captured_variables.rb:61:9:61:21 | return | instance_variables.rb:120:6:120:20 | call to get_field | | instance_variables.rb:120:6:120:10 | foo16 : Foo [@field] | instance_variables.rb:13:5:15:7 | self in get_field : Foo [@field] | instance_variables.rb:14:9:14:21 | return | instance_variables.rb:120:6:120:20 | call to get_field | testFailures +| callbacks.rb:18:25:18:25 | x | Unexpected result: hasValueFlow=1 | +| callbacks.rb:29:37:29:37 | x | Unexpected result: hasValueFlow=2 | #select | blocks.rb:8:10:8:14 | yield ... | blocks.rb:14:12:14:20 | call to source | blocks.rb:8:10:8:14 | yield ... | $@ | blocks.rb:14:12:14:20 | call to source | call to source | | blocks.rb:25:8:25:8 | x | blocks.rb:24:3:24:11 | call to source | blocks.rb:25:8:25:8 | x | $@ | blocks.rb:24:3:24:11 | call to source | call to source | +| callbacks.rb:17:25:17:25 | x | callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:17:25:17:25 | x | $@ | callbacks.rb:17:31:17:38 | call to taint | call to taint | +| callbacks.rb:18:25:18:25 | x | callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:18:25:18:25 | x | $@ | callbacks.rb:17:31:17:38 | call to taint | call to taint | +| callbacks.rb:28:39:28:39 | x | callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:28:39:28:39 | x | $@ | callbacks.rb:28:18:28:25 | call to taint | call to taint | +| callbacks.rb:29:37:29:37 | x | callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:29:37:29:37 | x | $@ | callbacks.rb:28:18:28:25 | call to taint | call to taint | | captured_variables.rb:10:20:10:20 | x | captured_variables.rb:13:20:13:29 | call to taint | captured_variables.rb:10:20:10:20 | x | $@ | captured_variables.rb:13:20:13:29 | call to taint | call to taint | | captured_variables.rb:17:14:17:14 | x | captured_variables.rb:20:25:20:34 | call to taint | captured_variables.rb:17:14:17:14 | x | $@ | captured_variables.rb:20:25:20:34 | call to taint | call to taint | | captured_variables.rb:24:14:24:14 | x | captured_variables.rb:27:48:27:57 | call to taint | captured_variables.rb:24:14:24:14 | x | $@ | captured_variables.rb:27:48:27:57 | call to taint | call to taint | diff --git a/ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected b/ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected index 413b5d6748f6..07e85877ca5d 100644 --- a/ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected +++ b/ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected @@ -1,4 +1,6 @@ | blocks.rb:4:10:4:10 | r | Fixed missing result: hasValueFlow=1 | +| callbacks.rb:17:41:17:58 | # $ hasValueFlow=1 | Missing result: hasValueFlow=1 | +| callbacks.rb:29:37:29:37 | x | Unexpected result: hasValueFlow=2 | | captured_variables.rb:50:10:50:10 | x | Fixed missing result: hasValueFlow=2 | | captured_variables.rb:68:25:68:68 | # $ hasValueFlow=3 $ MISSING: hasValueFlow=4 | Missing result: hasValueFlow=3 | | captured_variables.rb:72:21:72:66 | # $ hasValueFlow=4 $ SPURIOUS: hasValueFlow=3 | Fixed spurious result: hasValueFlow=3 | diff --git a/ruby/ql/test/library-tests/dataflow/global/callbacks.rb b/ruby/ql/test/library-tests/dataflow/global/callbacks.rb new file mode 100644 index 000000000000..e11747ca6d5e --- /dev/null +++ b/ruby/ql/test/library-tests/dataflow/global/callbacks.rb @@ -0,0 +1,29 @@ +def taint x + x +end + +def sink x + puts "SINK: #{x}" +end + +def apply (f, x) + f.call(x) +end + +def apply_wrap (f, x) + apply(f, x) +end + +apply_wrap(->(x) { sink(x) }, taint(1)) # $ hasValueFlow=1 +apply_wrap(->(x) { sink(x) }, "safe") + +def apply_block x + yield x +end + +def apply_block_wrap (x, &block) + apply_block(x, &block) +end + +apply_block_wrap(taint(2)) { |x| sink(x) } # $ hasValueFlow=2 +apply_block_wrap("safe") { |x| sink(x) } \ No newline at end of file From de0deabe4c8eddbf0fe0842f174aab479c4ee1d7 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Sat, 23 Sep 2023 14:16:51 +0200 Subject: [PATCH 2/2] Ruby: Implement `localMustFlowStep` --- .../dataflow/internal/DataFlowPrivate.qll | 21 ++++++++++++++++++- .../call-sensitivity.expected | 5 ----- .../call-sensitivity/call_sensitivity.rb | 2 +- .../dataflow/global/Flow.expected | 12 ----------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 794fd31cffc2..05af2d0c07e1 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -1972,7 +1972,26 @@ private predicate mustHaveCollectionType(Node n, DataFlowType t) { not n instanceof SynthSplatParameterNode } -predicate localMustFlowStep(Node node1, Node node2) { none() } +predicate localMustFlowStep(Node node1, Node node2) { + node1 = SsaFlow::toParameterNodeImpl(node2) + or + exists(SsaImpl::Definition def | + def.(Ssa::WriteDefinition).assigns(node1.asExpr()) and + node2.(SsaDefinitionExtNode).getDefinitionExt() = def + or + def = node1.(SsaDefinitionExtNode).getDefinitionExt() and + node2.asExpr() = SsaImpl::getARead(def) + ) + or + node1.asExpr() = node2.asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs() + or + node1.asExpr() = node2.asExpr().(CfgNodes::ExprNodes::BlockArgumentCfgNode).getValue() + or + node2.(ImplicitBlockArgumentNode).getParameterNode(_) = node1 + or + FlowSummaryImpl::Private::Steps::summaryLocalMustFlowStep(node1.(FlowSummaryNode).getSummaryNode(), + node2.(FlowSummaryNode).getSummaryNode()) +} /** Gets the type of `n` used for type pruning. */ DataFlowType getNodeType(Node n) { diff --git a/ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected b/ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected index 4e05b2c06fca..a6cb70d1df22 100644 --- a/ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected +++ b/ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected @@ -74,11 +74,9 @@ edges | call_sensitivity.rb:194:17:194:17 | x | call_sensitivity.rb:189:19:189:19 | x | provenance | | | call_sensitivity.rb:194:23:194:23 | x | call_sensitivity.rb:195:11:195:11 | x | provenance | | | call_sensitivity.rb:195:11:195:11 | x | call_sensitivity.rb:199:30:199:30 | x | provenance | | -| call_sensitivity.rb:195:11:195:11 | x | call_sensitivity.rb:203:26:203:26 | x | provenance | | | call_sensitivity.rb:199:15:199:24 | ( ... ) | call_sensitivity.rb:193:19:193:19 | x | provenance | | | call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:199:15:199:24 | ( ... ) | provenance | | | call_sensitivity.rb:199:30:199:30 | x | call_sensitivity.rb:200:8:200:8 | x | provenance | | -| call_sensitivity.rb:203:26:203:26 | x | call_sensitivity.rb:204:8:204:8 | x | provenance | | | call_sensitivity.rb:207:16:207:16 | y | call_sensitivity.rb:209:9:209:9 | y | provenance | | | call_sensitivity.rb:209:9:209:9 | y | call_sensitivity.rb:214:9:214:9 | x | provenance | | | call_sensitivity.rb:214:9:214:9 | x | call_sensitivity.rb:215:10:215:10 | x | provenance | | @@ -169,8 +167,6 @@ nodes | call_sensitivity.rb:199:16:199:23 | call to taint | semmle.label | call to taint | | call_sensitivity.rb:199:30:199:30 | x | semmle.label | x | | call_sensitivity.rb:200:8:200:8 | x | semmle.label | x | -| call_sensitivity.rb:203:26:203:26 | x | semmle.label | x | -| call_sensitivity.rb:204:8:204:8 | x | semmle.label | x | | call_sensitivity.rb:207:16:207:16 | y | semmle.label | y | | call_sensitivity.rb:209:9:209:9 | y | semmle.label | y | | call_sensitivity.rb:214:9:214:9 | x | semmle.label | x | @@ -204,7 +200,6 @@ testFailures | call_sensitivity.rb:105:10:105:10 | x | call_sensitivity.rb:178:11:178:19 | call to taint | call_sensitivity.rb:105:10:105:10 | x | $@ | call_sensitivity.rb:178:11:178:19 | call to taint | call to taint | | call_sensitivity.rb:105:10:105:10 | x | call_sensitivity.rb:187:12:187:19 | call to taint | call_sensitivity.rb:105:10:105:10 | x | $@ | call_sensitivity.rb:187:12:187:19 | call to taint | call to taint | | call_sensitivity.rb:200:8:200:8 | x | call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:200:8:200:8 | x | $@ | call_sensitivity.rb:199:16:199:23 | call to taint | call to taint | -| call_sensitivity.rb:204:8:204:8 | x | call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:204:8:204:8 | x | $@ | call_sensitivity.rb:199:16:199:23 | call to taint | call to taint | | call_sensitivity.rb:215:10:215:10 | x | call_sensitivity.rb:222:16:222:23 | call to taint | call_sensitivity.rb:215:10:215:10 | x | $@ | call_sensitivity.rb:222:16:222:23 | call to taint | call to taint | mayBenefitFromCallContext | call_sensitivity.rb:6:5:6:21 | call to puts | diff --git a/ruby/ql/test/library-tests/dataflow/call-sensitivity/call_sensitivity.rb b/ruby/ql/test/library-tests/dataflow/call-sensitivity/call_sensitivity.rb index 0e717794ddc6..fc628385bd48 100644 --- a/ruby/ql/test/library-tests/dataflow/call-sensitivity/call_sensitivity.rb +++ b/ruby/ql/test/library-tests/dataflow/call-sensitivity/call_sensitivity.rb @@ -201,7 +201,7 @@ def invoke_block2 x end invoke_block2 "safe" do |x| - sink x # $ SPURIOUS hasValueFlow=37 + sink x end def call_m (x, y) diff --git a/ruby/ql/test/library-tests/dataflow/global/Flow.expected b/ruby/ql/test/library-tests/dataflow/global/Flow.expected index 2272665cba3a..4ef28f2728a2 100644 --- a/ruby/ql/test/library-tests/dataflow/global/Flow.expected +++ b/ruby/ql/test/library-tests/dataflow/global/Flow.expected @@ -7,20 +7,16 @@ edges | blocks.rb:24:18:24:18 | x | blocks.rb:25:8:25:8 | x | provenance | | | callbacks.rb:9:15:9:15 | x | callbacks.rb:10:12:10:12 | x | provenance | | | callbacks.rb:10:12:10:12 | x | callbacks.rb:17:15:17:15 | x | provenance | | -| callbacks.rb:10:12:10:12 | x | callbacks.rb:18:15:18:15 | x | provenance | | | callbacks.rb:13:20:13:20 | x | callbacks.rb:14:14:14:14 | x | provenance | | | callbacks.rb:14:14:14:14 | x | callbacks.rb:9:15:9:15 | x | provenance | | | callbacks.rb:17:15:17:15 | x | callbacks.rb:17:25:17:25 | x | provenance | | | callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:13:20:13:20 | x | provenance | | -| callbacks.rb:18:15:18:15 | x | callbacks.rb:18:25:18:25 | x | provenance | | | callbacks.rb:20:17:20:17 | x | callbacks.rb:21:11:21:11 | x | provenance | | | callbacks.rb:21:11:21:11 | x | callbacks.rb:28:31:28:31 | x | provenance | | -| callbacks.rb:21:11:21:11 | x | callbacks.rb:29:29:29:29 | x | provenance | | | callbacks.rb:24:23:24:23 | x | callbacks.rb:25:17:25:17 | x | provenance | | | callbacks.rb:25:17:25:17 | x | callbacks.rb:20:17:20:17 | x | provenance | | | callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:24:23:24:23 | x | provenance | | | callbacks.rb:28:31:28:31 | x | callbacks.rb:28:39:28:39 | x | provenance | | -| callbacks.rb:29:29:29:29 | x | callbacks.rb:29:37:29:37 | x | provenance | | | captured_variables.rb:9:24:9:24 | x | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | provenance | | | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | captured_variables.rb:10:20:10:20 | x | provenance | | | captured_variables.rb:13:20:13:29 | call to taint | captured_variables.rb:9:24:9:24 | x | provenance | | @@ -295,8 +291,6 @@ nodes | callbacks.rb:17:15:17:15 | x | semmle.label | x | | callbacks.rb:17:25:17:25 | x | semmle.label | x | | callbacks.rb:17:31:17:38 | call to taint | semmle.label | call to taint | -| callbacks.rb:18:15:18:15 | x | semmle.label | x | -| callbacks.rb:18:25:18:25 | x | semmle.label | x | | callbacks.rb:20:17:20:17 | x | semmle.label | x | | callbacks.rb:21:11:21:11 | x | semmle.label | x | | callbacks.rb:24:23:24:23 | x | semmle.label | x | @@ -304,8 +298,6 @@ nodes | callbacks.rb:28:18:28:25 | call to taint | semmle.label | call to taint | | callbacks.rb:28:31:28:31 | x | semmle.label | x | | callbacks.rb:28:39:28:39 | x | semmle.label | x | -| callbacks.rb:29:29:29:29 | x | semmle.label | x | -| callbacks.rb:29:37:29:37 | x | semmle.label | x | | captured_variables.rb:9:24:9:24 | x | semmle.label | x | | captured_variables.rb:10:20:10:20 | x | semmle.label | x | | captured_variables.rb:11:5:11:6 | fn : [lambda] [captured x] | semmle.label | fn : [lambda] [captured x] | @@ -616,15 +608,11 @@ subpaths | instance_variables.rb:120:6:120:10 | foo16 : Foo [@field] | captured_variables.rb:60:5:62:7 | self in get_field : Foo [@field] | captured_variables.rb:61:9:61:21 | return | instance_variables.rb:120:6:120:20 | call to get_field | | instance_variables.rb:120:6:120:10 | foo16 : Foo [@field] | instance_variables.rb:13:5:15:7 | self in get_field : Foo [@field] | instance_variables.rb:14:9:14:21 | return | instance_variables.rb:120:6:120:20 | call to get_field | testFailures -| callbacks.rb:18:25:18:25 | x | Unexpected result: hasValueFlow=1 | -| callbacks.rb:29:37:29:37 | x | Unexpected result: hasValueFlow=2 | #select | blocks.rb:8:10:8:14 | yield ... | blocks.rb:14:12:14:20 | call to source | blocks.rb:8:10:8:14 | yield ... | $@ | blocks.rb:14:12:14:20 | call to source | call to source | | blocks.rb:25:8:25:8 | x | blocks.rb:24:3:24:11 | call to source | blocks.rb:25:8:25:8 | x | $@ | blocks.rb:24:3:24:11 | call to source | call to source | | callbacks.rb:17:25:17:25 | x | callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:17:25:17:25 | x | $@ | callbacks.rb:17:31:17:38 | call to taint | call to taint | -| callbacks.rb:18:25:18:25 | x | callbacks.rb:17:31:17:38 | call to taint | callbacks.rb:18:25:18:25 | x | $@ | callbacks.rb:17:31:17:38 | call to taint | call to taint | | callbacks.rb:28:39:28:39 | x | callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:28:39:28:39 | x | $@ | callbacks.rb:28:18:28:25 | call to taint | call to taint | -| callbacks.rb:29:37:29:37 | x | callbacks.rb:28:18:28:25 | call to taint | callbacks.rb:29:37:29:37 | x | $@ | callbacks.rb:28:18:28:25 | call to taint | call to taint | | captured_variables.rb:10:20:10:20 | x | captured_variables.rb:13:20:13:29 | call to taint | captured_variables.rb:10:20:10:20 | x | $@ | captured_variables.rb:13:20:13:29 | call to taint | call to taint | | captured_variables.rb:17:14:17:14 | x | captured_variables.rb:20:25:20:34 | call to taint | captured_variables.rb:17:14:17:14 | x | $@ | captured_variables.rb:20:25:20:34 | call to taint | call to taint | | captured_variables.rb:24:14:24:14 | x | captured_variables.rb:27:48:27:57 | call to taint | captured_variables.rb:24:14:24:14 | x | $@ | captured_variables.rb:27:48:27:57 | call to taint | call to taint |