Address review comments

This commit is contained in:
Tom Hvitved
2022-05-24 10:53:17 +02:00
parent ab46c075f7
commit daf81ae90d
3 changed files with 67 additions and 67 deletions

View File

@@ -257,14 +257,14 @@ module Content {
/** A value in a pair with a known key. */
class KnownPairValueContent extends PairValueContent, TKnownPairValueContent {
private ConstantValue cv;
private ConstantValue key;
KnownPairValueContent() { this = TKnownPairValueContent(cv) }
KnownPairValueContent() { this = TKnownPairValueContent(key) }
/** Gets the index in the collection. */
ConstantValue getIndex() { result = cv }
ConstantValue getIndex() { result = key }
override string toString() { result = "pair " + cv }
override string toString() { result = "pair " + key }
}
/** A value in a pair with an unknown key. */

View File

@@ -183,17 +183,17 @@ module Array {
/** A call to `[]` with a known index. */
private class ElementReferenceReadKnownSummary extends ElementReferenceReadSummary {
private ConstantValue cv;
private ConstantValue index;
ElementReferenceReadKnownSummary() {
this = methodName + "(" + cv.serialize() + ")" and
this = methodName + "(" + index.serialize() + ")" and
mc.getNumberOfArguments() = 1 and
cv = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
if methodName = "slice" then cv.isInt(_) else any()
index = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
if methodName = "slice" then index.isInt(_) else any()
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[self].Element[?," + cv.serialize() + "]" and
input = "Argument[self].Element[?," + index.serialize() + "]" and
output = "ReturnValue" and
preservesValue = true
}
@@ -308,20 +308,20 @@ module Array {
/** A call to `[]=` with a known index. */
private class ElementReferenceStoreKnownSummary extends ElementReferenceStoreSummary {
private ConstantValue cv;
private ConstantValue index;
ElementReferenceStoreKnownSummary() {
mc.getNumberOfArguments() = 2 and
cv = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
this = "[" + cv.serialize() + "]="
index = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
this = "[" + index.serialize() + "]="
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[1]" and
output = "Argument[self].Element[" + cv.serialize() + "]" and
output = "Argument[self].Element[" + index.serialize() + "]" and
preservesValue = true
or
input = "Argument[self].WithoutElement[" + cv.serialize() + "]" and
input = "Argument[self].WithoutElement[" + index.serialize() + "]" and
output = "Argument[self]" and
preservesValue = true
}
@@ -392,16 +392,16 @@ module Array {
}
private class AtKnownSummary extends AtSummary {
private ConstantValue cv;
private ConstantValue index;
AtKnownSummary() {
this = "at(" + cv.serialize() + "]" and
this = "at(" + index.serialize() + "]" and
mc.getNumberOfArguments() = 1 and
cv = DataFlow::Content::getKnownElementIndex(mc.getArgument(0))
index = DataFlow::Content::getKnownElementIndex(mc.getArgument(0))
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[self].Element[" + cv.serialize() + ",?]" and
input = "Argument[self].Element[" + index.serialize() + ",?]" and
output = "ReturnValue" and
preservesValue = true
}
@@ -537,11 +537,11 @@ module Array {
}
private class DeleteKnownSummary extends DeleteSummary {
private ConstantValue cv;
private ConstantValue index;
DeleteKnownSummary() {
this = "delete(" + cv.serialize() + ")" and
mc.getArgument(0).getConstantValue() = cv
this = "delete(" + index.serialize() + ")" and
mc.getArgument(0).getConstantValue() = index
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
@@ -549,21 +549,21 @@ module Array {
or
(
(
if cv.isInt(_)
if index.isInt(_)
then
// array indices may get shifted
input = "Argument[self].WithoutElement[" + cv.serialize() + "].Element[0..]" and
input = "Argument[self].WithoutElement[" + index.serialize() + "].Element[0..]" and
output = "Argument[self].Element[?]"
or
input = "Argument[self].WithoutElement[0..]" and
output = "Argument[self]"
else (
input = "Argument[self].WithoutElement[" + cv.serialize() + "]" and
input = "Argument[self].WithoutElement[" + index.serialize() + "]" and
output = "Argument[self]"
)
)
or
input = "Argument[self].Element[" + cv.serialize() + ",?]" and
input = "Argument[self].Element[" + index.serialize() + ",?]" and
output = "ReturnValue"
) and
preservesValue = true
@@ -795,17 +795,17 @@ module Array {
}
private class FetchKnownSummary extends FetchSummary {
ConstantValue cv;
ConstantValue index;
FetchKnownSummary() {
this = "fetch(" + cv.serialize() + ")" and
cv = mc.getArgument(0).getConstantValue() and
not cv.isInt(any(int i | i < 0))
this = "fetch(" + index.serialize() + ")" and
index = mc.getArgument(0).getConstantValue() and
not index.isInt(any(int i | i < 0))
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
(
input = "Argument[self].Element[?," + cv.serialize() + "]" and
input = "Argument[self].Element[?," + index.serialize() + "]" and
output = "ReturnValue"
or
input = "Argument[0]" and
@@ -821,8 +821,8 @@ module Array {
private class FetchUnknownSummary extends FetchSummary {
FetchUnknownSummary() {
this = "fetch(index)" and
not exists(ConstantValue cv |
cv = mc.getArgument(0).getConstantValue() and not cv.isInt(any(int i | i < 0))
not exists(ConstantValue index |
index = mc.getArgument(0).getConstantValue() and not index.isInt(any(int i | i < 0))
)
}
@@ -1867,9 +1867,9 @@ module Enumerable {
output = "ReturnValue.Element[?]" and
preservesValue = true
or
exists(ConstantValue cv |
not cv.isInt(_) and
input = "Argument[self].WithElement[" + cv.serialize() + "]" and
exists(ConstantValue index |
not index.isInt(_) and
input = "Argument[self].WithElement[" + index.serialize() + "]" and
output = "ReturnValue" and
preservesValue = true
)

View File

@@ -17,8 +17,8 @@ private import codeql.ruby.dataflow.internal.DataFlowDispatch
*/
module Hash {
// cannot use API graphs due to negative recursion
private predicate isHashLiteralPair(Pair pair, ConstantValue cv) {
cv = DataFlow::Content::getKnownElementIndex(pair.getKey()) and
private predicate isHashLiteralPair(Pair pair, ConstantValue key) {
key = DataFlow::Content::getKnownElementIndex(pair.getKey()) and
pair = any(MethodCall mc | mc.getMethodName() = "[]").getAnArgument()
}
@@ -44,23 +44,23 @@ module Hash {
}
private class HashLiteralNonSymbolSummary extends SummarizedCallable {
private ConstantValue cv;
private ConstantValue key;
HashLiteralNonSymbolSummary() {
this = "Hash.[]" and
isHashLiteralPair(_, cv) and
not cv.isSymbol(_)
isHashLiteralPair(_, key) and
not key.isSymbol(_)
}
final override MethodCall getACall() {
result = API::getTopLevelMember("Hash").getAMethodCall("[]").getExprNode().getExpr() and
isHashLiteralPair(result.getAnArgument(), cv)
isHashLiteralPair(result.getAnArgument(), key)
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
// { 'nonsymbol' => x }
input = "Argument[0..].PairValue[" + cv.serialize() + "]" and
output = "ReturnValue.Element[" + cv.serialize() + "]" and
input = "Argument[0..].PairValue[" + key.serialize() + "]" and
output = "ReturnValue.Element[" + key.serialize() + "]" and
preservesValue = true
}
}
@@ -110,27 +110,27 @@ module Hash {
*/
private class HashNewSuccessivePairsSummary extends SummarizedCallable {
private int i;
private ConstantValue cv;
private ConstantValue key;
HashNewSuccessivePairsSummary() {
this = "Hash[" + i + ", " + cv.serialize() + "]" and
this = "Hash[" + i + ", " + key.serialize() + "]" and
i % 2 = 1 and
exists(ElementReference er |
cv = er.getArgument(i - 1).getConstantValue() and
key = er.getArgument(i - 1).getConstantValue() and
exists(er.getArgument(i))
)
}
final override ElementReference getACall() {
result.getReceiver() = API::getTopLevelMember("Hash").getAUse().asExpr().getExpr() and
cv = result.getArgument(i - 1).getConstantValue() and
key = result.getArgument(i - 1).getConstantValue() and
exists(result.getArgument(i))
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
// Hash[:symbol, x]
input = "Argument[" + i + "]" and
output = "ReturnValue.Element[" + cv.serialize() + "]" and
output = "ReturnValue.Element[" + key.serialize() + "]" and
preservesValue = true
}
}
@@ -165,21 +165,21 @@ module Hash {
}
private class StoreKnownSummary extends StoreSummary {
private ConstantValue cv;
private ConstantValue key;
StoreKnownSummary() {
cv = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
this = "store(" + cv.serialize() + ")"
key = DataFlow::Content::getKnownElementIndex(mc.getArgument(0)) and
this = "store(" + key.serialize() + ")"
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
super.propagatesFlowExt(input, output, preservesValue)
or
input = "Argument[1]" and
output = "Argument[self].Element[" + cv.serialize() + "]" and
output = "Argument[self].Element[" + key.serialize() + "]" and
preservesValue = true
or
input = "Argument[self].WithoutElement[" + cv.serialize() + "]" and
input = "Argument[self].WithoutElement[" + key.serialize() + "]" and
output = "Argument[self]" and
preservesValue = true
}
@@ -210,17 +210,17 @@ module Hash {
}
private class AssocKnownSummary extends AssocSummary {
private ConstantValue cv;
private ConstantValue key;
AssocKnownSummary() {
this = "assoc(" + cv.serialize() + "]" and
not cv.isInt(_) and // exclude arrays
this = "assoc(" + key.serialize() + "]" and
not key.isInt(_) and // exclude arrays
mc.getNumberOfArguments() = 1 and
cv = DataFlow::Content::getKnownElementIndex(mc.getArgument(0))
key = DataFlow::Content::getKnownElementIndex(mc.getArgument(0))
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[self].Element[" + cv.serialize() + ",?]" and
input = "Argument[self].Element[" + key.serialize() + ",?]" and
output = "ReturnValue.Element[1]" and
preservesValue = true
}
@@ -325,18 +325,18 @@ abstract private class FetchValuesSummary extends SummarizedCallable {
}
private class FetchValuesKnownSummary extends FetchValuesSummary {
ConstantValue cv;
ConstantValue key;
FetchValuesKnownSummary() {
forex(Expr arg | arg = mc.getAnArgument() | exists(arg.getConstantValue())) and
cv = mc.getAnArgument().getConstantValue() and
this = "fetch_values(" + cv.serialize() + ")"
key = mc.getAnArgument().getConstantValue() and
this = "fetch_values(" + key.serialize() + ")"
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
super.propagatesFlowExt(input, output, preservesValue)
or
input = "Argument[self].Element[" + cv.serialize() + "]" and
input = "Argument[self].Element[" + key.serialize() + "]" and
output = "ReturnValue.Element[?]" and
preservesValue = true
}
@@ -407,16 +407,16 @@ abstract private class SliceSummary extends SummarizedCallable {
}
private class SliceKnownSummary extends SliceSummary {
ConstantValue cv;
ConstantValue key;
SliceKnownSummary() {
cv = mc.getAnArgument().getConstantValue() and
this = "slice(" + cv.serialize() + ")" and
not cv.isInt(_) // covered in `Array.qll`
key = mc.getAnArgument().getConstantValue() and
this = "slice(" + key.serialize() + ")" and
not key.isInt(_) // covered in `Array.qll`
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[self].WithElement[?," + cv.serialize() + "]" and
input = "Argument[self].WithElement[?," + key.serialize() + "]" and
output = "ReturnValue" and
preservesValue = true
}