Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruby: Do not distinguish between symbols and strings in hash keys #17880

Merged
merged 2 commits into from
Nov 1, 2024
Merged

Ruby: Do not distinguish between symbols and strings in hash keys #17880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f04a55e
Ruby: Update hash extension tests
hvitved Oct 31, 2024
6b60865
Ruby: Do not distinguish between symbols and strings in hash keys
hvitved Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,28 @@ class ContentSet extends TContentSet {
this.isAny() and
exists(result)
or
result = this.getAnElementReadContent()
exists(Content elementContent | elementContent = this.getAnElementReadContent() |
result = elementContent
or
// Do not distinguish symbol keys from string keys. This allows us to
// give more precise summaries for something like `with_indifferent_access`,
// and the amount of false-positive flow arising from this should be very
// limited.
elementContent =
any(Content::KnownElementContent known, ConstantValue cv |
cv = known.getIndex() and
result.(Content::KnownElementContent).getIndex() =
any(ConstantValue cv2 |
cv2.(ConstantValue::ConstantSymbolValue).getStringlikeValue() =
cv.(ConstantValue::ConstantStringValue).getStringlikeValue()
or
cv2.(ConstantValue::ConstantStringValue).getStringlikeValue() =
cv.(ConstantValue::ConstantSymbolValue).getStringlikeValue()
)
|
known
)
)
}
}

Expand Down
15 changes: 3 additions & 12 deletions ruby/ql/lib/codeql/ruby/frameworks/ActiveSupport.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,6 @@ module ActiveSupport {
* Extensions to the `Hash` class.
*/
module Hash {
private class WithIndifferentAccessSummary extends SimpleSummarizedCallable {
WithIndifferentAccessSummary() { this = "with_indifferent_access" }

override predicate propagatesFlow(string input, string output, boolean preservesValue) {
input = "Argument[self].Element[any]" and
output = "ReturnValue.Element[any]" and
preservesValue = true
}
}

/**
* Flow summary for `reverse_merge`, and its alias `with_defaults`.
*/
Expand Down Expand Up @@ -167,8 +157,9 @@ module ActiveSupport {
}

override predicate propagatesFlow(string input, string output, boolean preservesValue) {
input = "Argument[self].Element[any]" and
output = "ReturnValue.Element[?]" and
// keys are considered equal modulo string/symbol in our implementation
input = "Argument[self].WithElement[any]" and
output = "ReturnValue" and
preservesValue = true
}
}
Expand Down
44 changes: 44 additions & 0 deletions ruby/ql/test/library-tests/dataflow/flow-summaries/semantics.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,22 +458,40 @@ edges
| semantics.rb:263:14:263:14 | h [element :foo] | semantics.rb:263:10:263:15 | call to s31 | provenance | |
| semantics.rb:263:14:263:14 | h [element] | semantics.rb:263:10:263:15 | call to s31 | provenance | |
| semantics.rb:263:14:263:14 | h [element] | semantics.rb:263:10:263:15 | call to s31 | provenance | |
| semantics.rb:267:5:267:5 | [post] h [element :foo] | semantics.rb:268:5:268:5 | h [element :foo] | provenance | |
| semantics.rb:267:5:267:5 | [post] h [element :foo] | semantics.rb:268:5:268:5 | h [element :foo] | provenance | |
| semantics.rb:267:15:267:25 | call to source | semantics.rb:267:5:267:5 | [post] h [element :foo] | provenance | |
| semantics.rb:267:15:267:25 | call to source | semantics.rb:267:5:267:5 | [post] h [element :foo] | provenance | |
| semantics.rb:268:5:268:5 | [post] h [element :foo] | semantics.rb:269:5:269:5 | h [element :foo] | provenance | |
| semantics.rb:268:5:268:5 | [post] h [element :foo] | semantics.rb:269:5:269:5 | h [element :foo] | provenance | |
| semantics.rb:268:5:268:5 | [post] h [element foo] | semantics.rb:269:5:269:5 | h [element foo] | provenance | |
| semantics.rb:268:5:268:5 | [post] h [element foo] | semantics.rb:269:5:269:5 | h [element foo] | provenance | |
| semantics.rb:268:5:268:5 | h [element :foo] | semantics.rb:268:5:268:5 | [post] h [element :foo] | provenance | |
| semantics.rb:268:5:268:5 | h [element :foo] | semantics.rb:268:5:268:5 | [post] h [element :foo] | provenance | |
| semantics.rb:268:16:268:26 | call to source | semantics.rb:268:5:268:5 | [post] h [element foo] | provenance | |
| semantics.rb:268:16:268:26 | call to source | semantics.rb:268:5:268:5 | [post] h [element foo] | provenance | |
| semantics.rb:269:5:269:5 | [post] h [element :foo] | semantics.rb:270:5:270:5 | h [element :foo] | provenance | |
| semantics.rb:269:5:269:5 | [post] h [element :foo] | semantics.rb:270:5:270:5 | h [element :foo] | provenance | |
| semantics.rb:269:5:269:5 | [post] h [element foo] | semantics.rb:270:5:270:5 | h [element foo] | provenance | |
| semantics.rb:269:5:269:5 | [post] h [element foo] | semantics.rb:270:5:270:5 | h [element foo] | provenance | |
| semantics.rb:269:5:269:5 | h [element :foo] | semantics.rb:269:5:269:5 | [post] h [element :foo] | provenance | |
| semantics.rb:269:5:269:5 | h [element :foo] | semantics.rb:269:5:269:5 | [post] h [element :foo] | provenance | |
| semantics.rb:269:5:269:5 | h [element foo] | semantics.rb:269:5:269:5 | [post] h [element foo] | provenance | |
| semantics.rb:269:5:269:5 | h [element foo] | semantics.rb:269:5:269:5 | [post] h [element foo] | provenance | |
| semantics.rb:270:5:270:5 | [post] h [element :foo] | semantics.rb:273:14:273:14 | h [element :foo] | provenance | |
| semantics.rb:270:5:270:5 | [post] h [element :foo] | semantics.rb:273:14:273:14 | h [element :foo] | provenance | |
| semantics.rb:270:5:270:5 | [post] h [element foo] | semantics.rb:273:14:273:14 | h [element foo] | provenance | |
| semantics.rb:270:5:270:5 | [post] h [element foo] | semantics.rb:273:14:273:14 | h [element foo] | provenance | |
| semantics.rb:270:5:270:5 | h [element :foo] | semantics.rb:270:5:270:5 | [post] h [element :foo] | provenance | |
| semantics.rb:270:5:270:5 | h [element :foo] | semantics.rb:270:5:270:5 | [post] h [element :foo] | provenance | |
| semantics.rb:270:5:270:5 | h [element foo] | semantics.rb:270:5:270:5 | [post] h [element foo] | provenance | |
| semantics.rb:270:5:270:5 | h [element foo] | semantics.rb:270:5:270:5 | [post] h [element foo] | provenance | |
| semantics.rb:271:5:271:5 | [post] h [element] | semantics.rb:273:14:273:14 | h [element] | provenance | |
| semantics.rb:271:5:271:5 | [post] h [element] | semantics.rb:273:14:273:14 | h [element] | provenance | |
| semantics.rb:271:12:271:22 | call to source | semantics.rb:271:5:271:5 | [post] h [element] | provenance | |
| semantics.rb:271:12:271:22 | call to source | semantics.rb:271:5:271:5 | [post] h [element] | provenance | |
| semantics.rb:273:14:273:14 | h [element :foo] | semantics.rb:273:10:273:15 | call to s32 | provenance | |
| semantics.rb:273:14:273:14 | h [element :foo] | semantics.rb:273:10:273:15 | call to s32 | provenance | |
| semantics.rb:273:14:273:14 | h [element foo] | semantics.rb:273:10:273:15 | call to s32 | provenance | |
| semantics.rb:273:14:273:14 | h [element foo] | semantics.rb:273:10:273:15 | call to s32 | provenance | |
| semantics.rb:273:14:273:14 | h [element] | semantics.rb:273:10:273:15 | call to s32 | provenance | |
Expand Down Expand Up @@ -538,6 +556,8 @@ edges
| semantics.rb:291:10:291:10 | x [element :foo] | semantics.rb:291:10:291:16 | ...[...] | provenance | |
| semantics.rb:293:10:293:10 | x [element :foo] | semantics.rb:293:10:293:13 | ...[...] | provenance | |
| semantics.rb:293:10:293:10 | x [element :foo] | semantics.rb:293:10:293:13 | ...[...] | provenance | |
| semantics.rb:297:5:297:5 | x [element foo] | semantics.rb:298:10:298:10 | x [element foo] | provenance | |
| semantics.rb:297:5:297:5 | x [element foo] | semantics.rb:298:10:298:10 | x [element foo] | provenance | |
| semantics.rb:297:5:297:5 | x [element foo] | semantics.rb:299:10:299:10 | x [element foo] | provenance | |
| semantics.rb:297:5:297:5 | x [element foo] | semantics.rb:299:10:299:10 | x [element foo] | provenance | |
| semantics.rb:297:5:297:5 | x [element foo] | semantics.rb:301:10:301:10 | x [element foo] | provenance | |
Expand All @@ -546,6 +566,8 @@ edges
| semantics.rb:297:9:297:24 | call to s36 [element foo] | semantics.rb:297:5:297:5 | x [element foo] | provenance | |
| semantics.rb:297:13:297:23 | call to source | semantics.rb:297:9:297:24 | call to s36 [element foo] | provenance | |
| semantics.rb:297:13:297:23 | call to source | semantics.rb:297:9:297:24 | call to s36 [element foo] | provenance | |
| semantics.rb:298:10:298:10 | x [element foo] | semantics.rb:298:10:298:16 | ...[...] | provenance | |
| semantics.rb:298:10:298:10 | x [element foo] | semantics.rb:298:10:298:16 | ...[...] | provenance | |
| semantics.rb:299:10:299:10 | x [element foo] | semantics.rb:299:10:299:17 | ...[...] | provenance | |
| semantics.rb:299:10:299:10 | x [element foo] | semantics.rb:299:10:299:17 | ...[...] | provenance | |
| semantics.rb:301:10:301:10 | x [element foo] | semantics.rb:301:10:301:13 | ...[...] | provenance | |
Expand Down Expand Up @@ -1616,16 +1638,32 @@ nodes
| semantics.rb:263:14:263:14 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:263:14:263:14 | h [element] | semmle.label | h [element] |
| semantics.rb:263:14:263:14 | h [element] | semmle.label | h [element] |
| semantics.rb:267:5:267:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:267:5:267:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:267:15:267:25 | call to source | semmle.label | call to source |
| semantics.rb:267:15:267:25 | call to source | semmle.label | call to source |
| semantics.rb:268:5:268:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:268:5:268:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:268:5:268:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:268:5:268:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:268:5:268:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:268:5:268:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:268:16:268:26 | call to source | semmle.label | call to source |
| semantics.rb:268:16:268:26 | call to source | semmle.label | call to source |
| semantics.rb:269:5:269:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:269:5:269:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:269:5:269:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:269:5:269:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:269:5:269:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:269:5:269:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:269:5:269:5 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:269:5:269:5 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:270:5:270:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:270:5:270:5 | [post] h [element :foo] | semmle.label | [post] h [element :foo] |
| semantics.rb:270:5:270:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:270:5:270:5 | [post] h [element foo] | semmle.label | [post] h [element foo] |
| semantics.rb:270:5:270:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:270:5:270:5 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:270:5:270:5 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:270:5:270:5 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:271:5:271:5 | [post] h [element] | semmle.label | [post] h [element] |
Expand All @@ -1634,6 +1672,8 @@ nodes
| semantics.rb:271:12:271:22 | call to source | semmle.label | call to source |
| semantics.rb:273:10:273:15 | call to s32 | semmle.label | call to s32 |
| semantics.rb:273:10:273:15 | call to s32 | semmle.label | call to s32 |
| semantics.rb:273:14:273:14 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:273:14:273:14 | h [element :foo] | semmle.label | h [element :foo] |
| semantics.rb:273:14:273:14 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:273:14:273:14 | h [element foo] | semmle.label | h [element foo] |
| semantics.rb:273:14:273:14 | h [element] | semmle.label | h [element] |
Expand Down Expand Up @@ -1708,6 +1748,10 @@ nodes
| semantics.rb:297:9:297:24 | call to s36 [element foo] | semmle.label | call to s36 [element foo] |
| semantics.rb:297:13:297:23 | call to source | semmle.label | call to source |
| semantics.rb:297:13:297:23 | call to source | semmle.label | call to source |
| semantics.rb:298:10:298:10 | x [element foo] | semmle.label | x [element foo] |
| semantics.rb:298:10:298:10 | x [element foo] | semmle.label | x [element foo] |
| semantics.rb:298:10:298:16 | ...[...] | semmle.label | ...[...] |
| semantics.rb:298:10:298:16 | ...[...] | semmle.label | ...[...] |
| semantics.rb:299:10:299:10 | x [element foo] | semmle.label | x [element foo] |
| semantics.rb:299:10:299:10 | x [element foo] | semmle.label | x [element foo] |
| semantics.rb:299:10:299:17 | ...[...] | semmle.label | ...[...] |
Expand Down
4 changes: 2 additions & 2 deletions ruby/ql/test/library-tests/dataflow/flow-summaries/semantics.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ def m32(h, i)
h[1] = source("d")
h[i] = source("e")

sink s32(h) # $ hasValueFlow=b hasValueFlow=e
sink s32(h) # $ hasValueFlow=b $ hasValueFlow=e $ SPURIOUS: hasValueFlow=a
end

def m33(h, i)
Expand All @@ -295,7 +295,7 @@ def m35(h, i)

def m36(h, i)
x = s36(source("a"))
sink x[:foo]
sink x[:foo] # $ SPURIOUS: hasValueFlow=a
sink x["foo"] # $ hasValueFlow=a
sink x[:bar]
sink x[i] # $ hasValueFlow=a
Expand Down
Loading
Loading