Skip to content

Commit 97e939a

Browse files
committed
Ruby: refine summaries for Hash#reverse_merge etc.
- revert the changes to the taint summaries specific to ActionController params - make the general flow summaries value-preserving and use WithElement[any]
1 parent a9ff0bd commit 97e939a

File tree

5 files changed

+485
-6
lines changed

5 files changed

+485
-6
lines changed

ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,12 +592,15 @@ private module ParamsSummaries {
592592
/**
593593
* `#merge`
594594
* Returns a new ActionController::Parameters with all keys from other_hash merged into current hash.
595+
* `#reverse_merge`
596+
* `#with_defaults`
597+
* Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
595598
*/
596599
private class MergeSummary extends SummarizedCallable {
597600
MergeSummary() { this = "ActionController::Parameters#merge" }
598601

599602
override MethodCall getACall() {
600-
result.getMethodName() = "merge" and
603+
result.getMethodName() = ["merge", "reverse_merge", "with_defaults"] and
601604
exists(ParamsInstance i |
602605
i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
603606
)
@@ -613,12 +616,16 @@ private module ParamsSummaries {
613616
/**
614617
* `#merge!`
615618
* Returns current ActionController::Parameters instance with current hash merged into other_hash.
619+
* `#reverse_merge!`
620+
* `#with_defaults!`
621+
* `#reverse_update`
622+
* Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
616623
*/
617624
private class MergeBangSummary extends SummarizedCallable {
618625
MergeBangSummary() { this = "ActionController::Parameters#merge!" }
619626

620627
override MethodCall getACall() {
621-
result.getMethodName() = "merge!" and
628+
result.getMethodName() = ["merge!", "reverse_merge!", "with_defaults!", "reverse_update"] and
622629
exists(ParamsInstance i |
623630
i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
624631
)

ruby/ql/lib/codeql/ruby/frameworks/ActiveSupport.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,9 @@ module ActiveSupport {
127127
ReverseMergeSummary() { this = ["reverse_merge", "with_defaults"] }
128128

129129
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
130-
input = ["Argument[self]", "Argument[0]"] and
130+
input = "Argument[self,0].WithElement[any]" and
131131
output = "ReturnValue" and
132-
preservesValue = false
132+
preservesValue = true
133133
}
134134
}
135135

@@ -140,9 +140,9 @@ module ActiveSupport {
140140
ReverseMergeBangSummary() { this = ["reverse_merge!", "with_defaults!", "reverse_update"] }
141141

142142
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
143-
input = ["Argument[self]", "Argument[0]"] and
143+
input = "Argument[self,0].WithElement[any]" and
144144
output = ["ReturnValue", "Argument[self]"] and
145-
preservesValue = false
145+
preservesValue = true
146146
}
147147
}
148148

0 commit comments

Comments
 (0)