Skip to content

Commit 0e6735b

Browse files
hvitvedhvitved
authored
Merge pull request #10691 from hvitved/dataflow/conjunctive-clears
Data flow: Take conjunctive `With(out)Contents` into account in `prohibitsUseUseFlow`
2 parents 387e575 + 0beea9f commit 0e6735b

File tree

11 files changed

+434
-325
lines changed

11 files changed

+434
-325
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,27 @@ module Private {
750750
)
751751
}
752752

753+
/**
754+
* Holds if `p` can reach `n` in a summarized callable, using only value-preserving
755+
* local steps. `clearsOrExpects` records whether any node on the path from `p` to
756+
* `n` either clears or expects contents.
757+
*/
758+
private predicate paramReachesLocal(ParamNode p, Node n, boolean clearsOrExpects) {
759+
viableParam(_, _, _, p) and
760+
n = p and
761+
clearsOrExpects = false
762+
or
763+
exists(Node mid, boolean clearsOrExpectsMid |
764+
paramReachesLocal(p, mid, clearsOrExpectsMid) and
765+
summaryLocalStep(mid, n, true) and
766+
if
767+
summaryClearsContent(n, _) or
768+
summaryExpectsContent(n, _)
769+
then clearsOrExpects = true
770+
else clearsOrExpects = clearsOrExpectsMid
771+
)
772+
}
773+
753774
/**
754775
* Holds if use-use flow starting from `arg` should be prohibited.
755776
*
@@ -759,15 +780,11 @@ module Private {
759780
*/
760781
pragma[nomagic]
761782
predicate prohibitsUseUseFlow(ArgNode arg, SummarizedCallable sc) {
762-
exists(ParamNode p, Node mid, ParameterPosition ppos, Node ret |
783+
exists(ParamNode p, ParameterPosition ppos, Node ret |
784+
paramReachesLocal(p, ret, true) and
763785
p = summaryArgParam0(_, arg, sc) and
764786
p.isParameterOf(_, pragma[only_bind_into](ppos)) and
765-
summaryLocalStep(p, mid, true) and
766-
summaryLocalStep(mid, ret, true) and
767787
isParameterPostUpdate(ret, _, pragma[only_bind_into](ppos))
768-
|
769-
summaryClearsContent(mid, _) or
770-
summaryExpectsContent(mid, _)
771788
)
772789
}
773790

‎java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,27 @@ module Private {
750750
)
751751
}
752752

753+
/**
754+
* Holds if `p` can reach `n` in a summarized callable, using only value-preserving
755+
* local steps. `clearsOrExpects` records whether any node on the path from `p` to
756+
* `n` either clears or expects contents.
757+
*/
758+
private predicate paramReachesLocal(ParamNode p, Node n, boolean clearsOrExpects) {
759+
viableParam(_, _, _, p) and
760+
n = p and
761+
clearsOrExpects = false
762+
or
763+
exists(Node mid, boolean clearsOrExpectsMid |
764+
paramReachesLocal(p, mid, clearsOrExpectsMid) and
765+
summaryLocalStep(mid, n, true) and
766+
if
767+
summaryClearsContent(n, _) or
768+
summaryExpectsContent(n, _)
769+
then clearsOrExpects = true
770+
else clearsOrExpects = clearsOrExpectsMid
771+
)
772+
}
773+
753774
/**
754775
* Holds if use-use flow starting from `arg` should be prohibited.
755776
*
@@ -759,15 +780,11 @@ module Private {
759780
*/
760781
pragma[nomagic]
761782
predicate prohibitsUseUseFlow(ArgNode arg, SummarizedCallable sc) {
762-
exists(ParamNode p, Node mid, ParameterPosition ppos, Node ret |
783+
exists(ParamNode p, ParameterPosition ppos, Node ret |
784+
paramReachesLocal(p, ret, true) and
763785
p = summaryArgParam0(_, arg, sc) and
764786
p.isParameterOf(_, pragma[only_bind_into](ppos)) and
765-
summaryLocalStep(p, mid, true) and
766-
summaryLocalStep(mid, ret, true) and
767787
isParameterPostUpdate(ret, _, pragma[only_bind_into](ppos))
768-
|
769-
summaryClearsContent(mid, _) or
770-
summaryExpectsContent(mid, _)
771788
)
772789
}
773790

‎python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImpl.qll‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,27 @@ module Private {
750750
)
751751
}
752752

753+
/**
754+
* Holds if `p` can reach `n` in a summarized callable, using only value-preserving
755+
* local steps. `clearsOrExpects` records whether any node on the path from `p` to
756+
* `n` either clears or expects contents.
757+
*/
758+
private predicate paramReachesLocal(ParamNode p, Node n, boolean clearsOrExpects) {
759+
viableParam(_, _, _, p) and
760+
n = p and
761+
clearsOrExpects = false
762+
or
763+
exists(Node mid, boolean clearsOrExpectsMid |
764+
paramReachesLocal(p, mid, clearsOrExpectsMid) and
765+
summaryLocalStep(mid, n, true) and
766+
if
767+
summaryClearsContent(n, _) or
768+
summaryExpectsContent(n, _)
769+
then clearsOrExpects = true
770+
else clearsOrExpects = clearsOrExpectsMid
771+
)
772+
}
773+
753774
/**
754775
* Holds if use-use flow starting from `arg` should be prohibited.
755776
*
@@ -759,15 +780,11 @@ module Private {
759780
*/
760781
pragma[nomagic]
761782
predicate prohibitsUseUseFlow(ArgNode arg, SummarizedCallable sc) {
762-
exists(ParamNode p, Node mid, ParameterPosition ppos, Node ret |
783+
exists(ParamNode p, ParameterPosition ppos, Node ret |
784+
paramReachesLocal(p, ret, true) and
763785
p = summaryArgParam0(_, arg, sc) and
764786
p.isParameterOf(_, pragma[only_bind_into](ppos)) and
765-
summaryLocalStep(p, mid, true) and
766-
summaryLocalStep(mid, ret, true) and
767787
isParameterPostUpdate(ret, _, pragma[only_bind_into](ppos))
768-
|
769-
summaryClearsContent(mid, _) or
770-
summaryExpectsContent(mid, _)
771788
)
772789
}
773790

‎ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,27 @@ module Private {
750750
)
751751
}
752752

753+
/**
754+
* Holds if `p` can reach `n` in a summarized callable, using only value-preserving
755+
* local steps. `clearsOrExpects` records whether any node on the path from `p` to
756+
* `n` either clears or expects contents.
757+
*/
758+
private predicate paramReachesLocal(ParamNode p, Node n, boolean clearsOrExpects) {
759+
viableParam(_, _, _, p) and
760+
n = p and
761+
clearsOrExpects = false
762+
or
763+
exists(Node mid, boolean clearsOrExpectsMid |
764+
paramReachesLocal(p, mid, clearsOrExpectsMid) and
765+
summaryLocalStep(mid, n, true) and
766+
if
767+
summaryClearsContent(n, _) or
768+
summaryExpectsContent(n, _)
769+
then clearsOrExpects = true
770+
else clearsOrExpects = clearsOrExpectsMid
771+
)
772+
}
773+
753774
/**
754775
* Holds if use-use flow starting from `arg` should be prohibited.
755776
*
@@ -759,15 +780,11 @@ module Private {
759780
*/
760781
pragma[nomagic]
761782
predicate prohibitsUseUseFlow(ArgNode arg, SummarizedCallable sc) {
762-
exists(ParamNode p, Node mid, ParameterPosition ppos, Node ret |
783+
exists(ParamNode p, ParameterPosition ppos, Node ret |
784+
paramReachesLocal(p, ret, true) and
763785
p = summaryArgParam0(_, arg, sc) and
764786
p.isParameterOf(_, pragma[only_bind_into](ppos)) and
765-
summaryLocalStep(p, mid, true) and
766-
summaryLocalStep(mid, ret, true) and
767787
isParameterPostUpdate(ret, _, pragma[only_bind_into](ppos))
768-
|
769-
summaryClearsContent(mid, _) or
770-
summaryExpectsContent(mid, _)
771788
)
772789
}
773790

‎ruby/ql/lib/codeql/ruby/frameworks/core/Hash.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ module Hash {
267267
s = getExceptComponent(mc, i)
268268
|
269269
".WithoutElement[" + s + "!]" order by i
270-
) and
270+
) + ".WithElement[any]" and
271271
output = "ReturnValue" and
272272
preservesValue = true
273273
}

‎ruby/ql/test/library-tests/dataflow/array-flow/array-flow.expected‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,6 @@ edges
564564
| array_flow.rb:334:10:334:10 | a [element] : | array_flow.rb:334:10:334:13 | ...[...] |
565565
| array_flow.rb:338:16:338:25 | call to source : | array_flow.rb:339:9:339:9 | a [element 2] : |
566566
| array_flow.rb:338:16:338:25 | call to source : | array_flow.rb:339:9:339:9 | a [element 2] : |
567-
| array_flow.rb:338:16:338:25 | call to source : | array_flow.rb:345:10:345:10 | a [element 2] : |
568-
| array_flow.rb:338:16:338:25 | call to source : | array_flow.rb:345:10:345:10 | a [element 2] : |
569567
| array_flow.rb:339:9:339:9 | [post] a [element] : | array_flow.rb:343:10:343:10 | a [element] : |
570568
| array_flow.rb:339:9:339:9 | [post] a [element] : | array_flow.rb:343:10:343:10 | a [element] : |
571569
| array_flow.rb:339:9:339:9 | [post] a [element] : | array_flow.rb:344:10:344:10 | a [element] : |
@@ -588,8 +586,6 @@ edges
588586
| array_flow.rb:343:10:343:10 | a [element] : | array_flow.rb:343:10:343:13 | ...[...] |
589587
| array_flow.rb:344:10:344:10 | a [element] : | array_flow.rb:344:10:344:13 | ...[...] |
590588
| array_flow.rb:344:10:344:10 | a [element] : | array_flow.rb:344:10:344:13 | ...[...] |
591-
| array_flow.rb:345:10:345:10 | a [element 2] : | array_flow.rb:345:10:345:13 | ...[...] |
592-
| array_flow.rb:345:10:345:10 | a [element 2] : | array_flow.rb:345:10:345:13 | ...[...] |
593589
| array_flow.rb:345:10:345:10 | a [element] : | array_flow.rb:345:10:345:13 | ...[...] |
594590
| array_flow.rb:345:10:345:10 | a [element] : | array_flow.rb:345:10:345:13 | ...[...] |
595591
| array_flow.rb:349:16:349:25 | call to source : | array_flow.rb:350:9:350:9 | a [element 2] : |
@@ -4098,8 +4094,6 @@ nodes
40984094
| array_flow.rb:344:10:344:10 | a [element] : | semmle.label | a [element] : |
40994095
| array_flow.rb:344:10:344:13 | ...[...] | semmle.label | ...[...] |
41004096
| array_flow.rb:344:10:344:13 | ...[...] | semmle.label | ...[...] |
4101-
| array_flow.rb:345:10:345:10 | a [element 2] : | semmle.label | a [element 2] : |
4102-
| array_flow.rb:345:10:345:10 | a [element 2] : | semmle.label | a [element 2] : |
41034097
| array_flow.rb:345:10:345:10 | a [element] : | semmle.label | a [element] : |
41044098
| array_flow.rb:345:10:345:10 | a [element] : | semmle.label | a [element] : |
41054099
| array_flow.rb:345:10:345:13 | ...[...] | semmle.label | ...[...] |

‎ruby/ql/test/library-tests/dataflow/hash-flow/hash-flow.expected‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ edges
102102
| hash_flow.rb:185:9:185:12 | hash [element :a] : | hash_flow.rb:185:9:185:23 | call to delete : |
103103
| hash_flow.rb:185:9:185:23 | call to delete : | hash_flow.rb:186:10:186:10 | a |
104104
| hash_flow.rb:194:15:194:25 | call to taint : | hash_flow.rb:197:9:197:12 | hash [element :a] : |
105-
| hash_flow.rb:194:15:194:25 | call to taint : | hash_flow.rb:202:10:202:13 | hash [element :a] : |
106105
| hash_flow.rb:197:9:197:12 | [post] hash [element :a] : | hash_flow.rb:202:10:202:13 | hash [element :a] : |
107106
| hash_flow.rb:197:9:197:12 | hash [element :a] : | hash_flow.rb:197:9:197:12 | [post] hash [element :a] : |
108107
| hash_flow.rb:197:9:197:12 | hash [element :a] : | hash_flow.rb:197:9:200:7 | call to delete_if [element :a] : |
@@ -307,7 +306,6 @@ edges
307306
| hash_flow.rb:477:29:477:33 | value : | hash_flow.rb:479:14:479:18 | value |
308307
| hash_flow.rb:482:10:482:10 | b [element :a] : | hash_flow.rb:482:10:482:14 | ...[...] |
309308
| hash_flow.rb:489:15:489:25 | call to taint : | hash_flow.rb:492:9:492:12 | hash [element :a] : |
310-
| hash_flow.rb:489:15:489:25 | call to taint : | hash_flow.rb:498:10:498:13 | hash [element :a] : |
311309
| hash_flow.rb:492:9:492:12 | [post] hash [element :a] : | hash_flow.rb:498:10:498:13 | hash [element :a] : |
312310
| hash_flow.rb:492:9:492:12 | hash [element :a] : | hash_flow.rb:492:9:492:12 | [post] hash [element :a] : |
313311
| hash_flow.rb:492:9:492:12 | hash [element :a] : | hash_flow.rb:492:9:496:7 | call to reject! [element :a] : |

0 commit comments

Comments
 (0)