Skip to content

Ruby: Remove PairValueContent #10686

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.

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

Merged
merged 1 commit into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 0 additions & 10 deletions ruby/ql/lib/codeql/ruby/dataflow/FlowSummary.qll
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,6 @@ module SummaryComponent {
result = SC::content(TElementLowerBoundContent(lower, true))
}

/** Gets a summary component that represents a value in a pair at an unknown key. */
SummaryComponent pairValueUnknown() {
result = SC::content(TSingletonContent(TUnknownPairValueContent()))
}

/** Gets a summary component that represents a value in a pair at a known key. */
SummaryComponent pairValueKnown(ConstantValue key) {
result = SC::content(TSingletonContent(DataFlow::Content::getPairValueContent(key)))
}

/** Gets a summary component that represents the return value of a call. */
SummaryComponent return() { result = SC::return(any(NormalReturnKind rk)) }
}
Expand Down
41 changes: 19 additions & 22 deletions ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ private module Cached {
} or
TSynthHashSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) {
exists(Argument arg | arg.isArgumentOf(c, any(ArgumentPosition pos | pos.isKeyword(_))))
or
c.getAnArgument() instanceof CfgNodes::ExprNodes::PairCfgNode
}

class TParameterNode =
Expand Down Expand Up @@ -405,8 +407,6 @@ private module Cached {
newtype TContent =
TKnownElementContent(ConstantValue cv) { trackKnownValue(cv) } or
TUnknownElementContent() or
TKnownPairValueContent(ConstantValue cv) { trackKnownValue(cv) } or
TUnknownPairValueContent() or
TFieldContent(string name) {
name = any(InstanceVariable v).getName()
or
Expand Down Expand Up @@ -443,8 +443,6 @@ private module Cached {

class TElementContent = TKnownElementContent or TUnknownElementContent;

class TPairValueContent = TKnownPairValueContent or TUnknownPairValueContent;

import Cached

/** Holds if `n` should be hidden from path explanations. */
Expand Down Expand Up @@ -1036,25 +1034,24 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
or
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
or
// Needed for pairs passed into method calls where the key is not a symbol,
// that is, where it is not a keyword argument.
node2.asExpr() =
any(CfgNodes::ExprNodes::PairCfgNode pair |
exists(CfgNodes::ExprCfgNode key, ConstantValue cv |
key = pair.getKey() and
pair.getValue() = node1.asExpr() and
cv = key.getConstantValue() and
not cv.isSymbol(_) and // handled as a keyword argument
c.isSingleton(Content::getPairValueContent(cv))
)
// Wrap all key-value arguments in a synthesized hash-splat argument node
exists(CfgNodes::ExprNodes::CallCfgNode call | node2 = TSynthHashSplatArgumentNode(call) |
// symbol key
exists(ArgumentPosition keywordPos, string name |
node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
keywordPos.isKeyword(name) and
c = getKeywordContent(name)
)
or
// non-symbol key
exists(CfgNodes::ExprNodes::PairCfgNode pair, CfgNodes::ExprCfgNode key, ConstantValue cv |
node1.asExpr() = pair.getValue() and
pair = call.getAnArgument() and
key = pair.getKey() and
cv = key.getConstantValue() and
not cv.isSymbol(_) and
c.isSingleton(TKnownElementContent(cv))
)
or
// Wrap all keyword arguments in a synthesized hash-splat argument node
exists(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition keywordPos, string name |
node2 = TSynthHashSplatArgumentNode(call) and
node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
keywordPos.isKeyword(name) and
c = getKeywordContent(name)
)
}

Expand Down
28 changes: 0 additions & 28 deletions ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
Original file line number Diff line number Diff line change
Expand Up @@ -295,34 +295,6 @@ module Content {
result = getElementContent(e.getConstantValue()).(KnownElementContent).getIndex()
}

/** A value in a pair with a known or unknown key. */
class PairValueContent extends Content, TPairValueContent { }

/** A value in a pair with a known key. */
class KnownPairValueContent extends PairValueContent, TKnownPairValueContent {
private ConstantValue key;

KnownPairValueContent() { this = TKnownPairValueContent(key) }

/** Gets the index in the collection. */
ConstantValue getIndex() { result = key }

override string toString() { result = "pair " + key }
}

/** A value in a pair with an unknown key. */
class UnknownPairValueContent extends PairValueContent, TUnknownPairValueContent {
override string toString() { result = "pair" }
}

/** Gets the content representing a value in a pair corresponding to constant value `cv`. */
PairValueContent getPairValueContent(ConstantValue cv) {
result = TKnownPairValueContent(cv)
or
not exists(TKnownPairValueContent(cv)) and
result = TUnknownPairValueContent()
}

/**
* A value stored behind a getter/setter pair.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,6 @@ SummaryComponent interpretComponentSpecific(AccessPathToken c) {
interpretElementArg(c.getAnArgument("WithoutElement")) and
result = FlowSummary::SummaryComponent::withoutContent(cs)
)
or
exists(string arg | arg = c.getAnArgument("PairValue") |
arg = "?" and
result = FlowSummary::SummaryComponent::pairValueUnknown()
or
exists(ConstantValue cv |
result = FlowSummary::SummaryComponent::pairValueKnown(cv) and
cv.serialize() = arg
)
)
}

/** Gets the textual representation of a summary component in the format used for flow summaries. */
Expand Down
28 changes: 0 additions & 28 deletions ruby/ql/lib/codeql/ruby/frameworks/core/Hash.qll
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,6 @@ private import codeql.ruby.ast.internal.Module
* in `Array.qll`.
*/
module Hash {
/**
* Holds if `key` is used as the non-symbol key in a hash literal. For example
*
* ```rb
* {
* :a => 1, # symbol
* "b" => 2 # non-symbol, "b" is the key
* }
* ```
*/
private predicate isHashLiteralNonSymbolKey(ConstantValue key) {
exists(Pair pair |
key = DataFlow::Content::getKnownElementIndex(pair.getKey()) and
// cannot use API graphs due to negative recursion
pair = any(MethodCall mc | mc.getMethodName() = "[]").getAnArgument() and
not key.isSymbol(_)
)
}

/**
* Gets a call to the method `name` invoked on the `Hash` object
* (not on a hash instance).
Expand All @@ -51,15 +32,6 @@ module Hash {
final override MethodCall getACallSimple() { result = getAStaticHashCall("[]") }

override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
// { 'nonsymbol' => x }
exists(ConstantValue key |
isHashLiteralNonSymbolKey(key) and
input = "Argument[0..].PairValue[" + key.serialize() + "]" and
output = "ReturnValue.Element[" + key.serialize() + "]" and
preservesValue = true
)
or
// { symbol: x }
// we make use of the special `hash-splat` argument kind, which contains all keyword
// arguments wrapped in an implicit hash, as well as explicit hash splat arguments
input = "Argument[hash-splat].WithElement[any]" and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ failures
edges
| hash_flow.rb:11:15:11:24 | call to taint : | hash_flow.rb:22:10:22:13 | hash [element :a] : |
| hash_flow.rb:13:12:13:21 | call to taint : | hash_flow.rb:24:10:24:13 | hash [element :c] : |
| hash_flow.rb:15:9:15:23 | Pair [pair e] : | hash_flow.rb:26:10:26:13 | hash [element e] : |
| hash_flow.rb:15:14:15:23 | call to taint : | hash_flow.rb:15:9:15:23 | Pair [pair e] : |
| hash_flow.rb:17:9:17:25 | Pair [pair g] : | hash_flow.rb:28:10:28:13 | hash [element g] : |
| hash_flow.rb:17:16:17:25 | call to taint : | hash_flow.rb:17:9:17:25 | Pair [pair g] : |
| hash_flow.rb:19:9:19:23 | Pair [pair 0] : | hash_flow.rb:30:10:30:13 | hash [element 0] : |
| hash_flow.rb:19:14:19:23 | call to taint : | hash_flow.rb:19:9:19:23 | Pair [pair 0] : |
| hash_flow.rb:15:14:15:23 | call to taint : | hash_flow.rb:26:10:26:13 | hash [element e] : |
| hash_flow.rb:17:16:17:25 | call to taint : | hash_flow.rb:28:10:28:13 | hash [element g] : |
| hash_flow.rb:19:14:19:23 | call to taint : | hash_flow.rb:30:10:30:13 | hash [element 0] : |
| hash_flow.rb:22:10:22:13 | hash [element :a] : | hash_flow.rb:22:10:22:17 | ...[...] |
| hash_flow.rb:24:10:24:13 | hash [element :c] : | hash_flow.rb:24:10:24:17 | ...[...] |
| hash_flow.rb:26:10:26:13 | hash [element e] : | hash_flow.rb:26:10:26:18 | ...[...] |
Expand Down Expand Up @@ -56,12 +53,10 @@ edges
| hash_flow.rb:68:22:68:31 | call to taint : | hash_flow.rb:68:13:68:39 | ...[...] [element :a] : |
| hash_flow.rb:69:10:69:14 | hash4 [element :a] : | hash_flow.rb:69:10:69:18 | ...[...] |
| hash_flow.rb:72:13:72:45 | ...[...] [element a] : | hash_flow.rb:73:10:73:14 | hash5 [element a] : |
| hash_flow.rb:72:18:72:34 | Pair [pair a] : | hash_flow.rb:72:13:72:45 | ...[...] [element a] : |
| hash_flow.rb:72:25:72:34 | call to taint : | hash_flow.rb:72:18:72:34 | Pair [pair a] : |
| hash_flow.rb:72:25:72:34 | call to taint : | hash_flow.rb:72:13:72:45 | ...[...] [element a] : |
| hash_flow.rb:73:10:73:14 | hash5 [element a] : | hash_flow.rb:73:10:73:19 | ...[...] |
| hash_flow.rb:76:13:76:47 | ...[...] [element a] : | hash_flow.rb:77:10:77:14 | hash6 [element a] : |
| hash_flow.rb:76:19:76:35 | Pair [pair a] : | hash_flow.rb:76:13:76:47 | ...[...] [element a] : |
| hash_flow.rb:76:26:76:35 | call to taint : | hash_flow.rb:76:19:76:35 | Pair [pair a] : |
| hash_flow.rb:76:26:76:35 | call to taint : | hash_flow.rb:76:13:76:47 | ...[...] [element a] : |
| hash_flow.rb:77:10:77:14 | hash6 [element a] : | hash_flow.rb:77:10:77:19 | ...[...] |
| hash_flow.rb:84:13:84:42 | call to [] [element :a] : | hash_flow.rb:85:10:85:14 | hash1 [element :a] : |
| hash_flow.rb:84:26:84:35 | call to taint : | hash_flow.rb:84:13:84:42 | call to [] [element :a] : |
Expand Down Expand Up @@ -533,11 +528,8 @@ edges
nodes
| hash_flow.rb:11:15:11:24 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:13:12:13:21 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:15:9:15:23 | Pair [pair e] : | semmle.label | Pair [pair e] : |
| hash_flow.rb:15:14:15:23 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:17:9:17:25 | Pair [pair g] : | semmle.label | Pair [pair g] : |
| hash_flow.rb:17:16:17:25 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:19:9:19:23 | Pair [pair 0] : | semmle.label | Pair [pair 0] : |
| hash_flow.rb:19:14:19:23 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:22:10:22:13 | hash [element :a] : | semmle.label | hash [element :a] : |
| hash_flow.rb:22:10:22:17 | ...[...] | semmle.label | ...[...] |
Expand Down Expand Up @@ -599,12 +591,10 @@ nodes
| hash_flow.rb:69:10:69:14 | hash4 [element :a] : | semmle.label | hash4 [element :a] : |
| hash_flow.rb:69:10:69:18 | ...[...] | semmle.label | ...[...] |
| hash_flow.rb:72:13:72:45 | ...[...] [element a] : | semmle.label | ...[...] [element a] : |
| hash_flow.rb:72:18:72:34 | Pair [pair a] : | semmle.label | Pair [pair a] : |
| hash_flow.rb:72:25:72:34 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:73:10:73:14 | hash5 [element a] : | semmle.label | hash5 [element a] : |
| hash_flow.rb:73:10:73:19 | ...[...] | semmle.label | ...[...] |
| hash_flow.rb:76:13:76:47 | ...[...] [element a] : | semmle.label | ...[...] [element a] : |
| hash_flow.rb:76:19:76:35 | Pair [pair a] : | semmle.label | Pair [pair a] : |
| hash_flow.rb:76:26:76:35 | call to taint : | semmle.label | call to taint : |
| hash_flow.rb:77:10:77:14 | hash6 [element a] : | semmle.label | hash6 [element a] : |
| hash_flow.rb:77:10:77:19 | ...[...] | semmle.label | ...[...] |
Expand Down