Skip to content

Commit 3ec43db

Browse files
hvitvedhvitved
committed
Ruby: Do not attempt to track precise hash indices for floats and complex numbers
1 parent 6e1914a commit 3ec43db

File tree

5 files changed

+124
-17
lines changed

5 files changed

+124
-17
lines changed

‎ruby/ql/lib/codeql/ruby/dataflow/FlowSummary.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ module SummaryComponent {
9191

9292
/** Gets a summary component that represents a value in a pair at a known key. */
9393
SummaryComponent pairValueKnown(ConstantValue key) {
94-
result = SC::content(TSingletonContent(TKnownPairValueContent(key)))
94+
result = SC::content(TSingletonContent(DataFlow::Content::getPairValueContent(key)))
9595
}
9696

9797
/** Gets a summary component that represents the return value of a call. */

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -392,14 +392,20 @@ private module Cached {
392392
TSingletonContent or TAnyElementContent or TKnownOrUnknownElementContent or
393393
TElementLowerBoundContent;
394394

395-
cached
396-
newtype TContent =
397-
TKnownElementContent(ConstantValue cv) {
395+
private predicate trackKnownValue(ConstantValue cv) {
396+
not cv.isFloat(_) and
397+
not cv.isComplex(_, _) and
398+
(
398399
not cv.isInt(_) or
399400
cv.getInt() in [0 .. 10]
400-
} or
401+
)
402+
}
403+
404+
cached
405+
newtype TContent =
406+
TKnownElementContent(ConstantValue cv) { trackKnownValue(cv) } or
401407
TUnknownElementContent() or
402-
TKnownPairValueContent(ConstantValue cv) or
408+
TKnownPairValueContent(ConstantValue cv) { trackKnownValue(cv) } or
403409
TUnknownPairValueContent() or
404410
TFieldContent(string name) {
405411
name = any(InstanceVariable v).getName()
@@ -1034,18 +1040,12 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
10341040
// that is, where it is not a keyword argument.
10351041
node2.asExpr() =
10361042
any(CfgNodes::ExprNodes::PairCfgNode pair |
1037-
exists(CfgNodes::ExprCfgNode key |
1043+
exists(CfgNodes::ExprCfgNode key, ConstantValue cv |
10381044
key = pair.getKey() and
1039-
pair.getValue() = node1.asExpr()
1040-
|
1041-
exists(ConstantValue cv |
1042-
cv = key.getConstantValue() and
1043-
not cv.isSymbol(_) and // handled as a keyword argument
1044-
c.isSingleton(TKnownPairValueContent(cv))
1045-
)
1046-
or
1047-
not exists(key.getConstantValue()) and
1048-
c.isSingleton(TUnknownPairValueContent())
1045+
pair.getValue() = node1.asExpr() and
1046+
cv = key.getConstantValue() and
1047+
not cv.isSymbol(_) and // handled as a keyword argument
1048+
c.isSingleton(Content::getPairValueContent(cv))
10491049
)
10501050
)
10511051
or

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,14 @@ module Content {
315315
override string toString() { result = "pair" }
316316
}
317317

318+
/** Gets the content representing a value in a pair corresponding to constant value `cv`. */
319+
PairValueContent getPairValueContent(ConstantValue cv) {
320+
result = TKnownPairValueContent(cv)
321+
or
322+
not exists(TKnownPairValueContent(cv)) and
323+
result = TUnknownPairValueContent()
324+
}
325+
318326
/**
319327
* A value stored behind a getter/setter pair.
320328
*

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3395,6 +3395,46 @@ edges
33953395
| array_flow.rb:1615:10:1615:10 | a [element, element 0] : | array_flow.rb:1615:10:1615:13 | ...[...] [element 0] : |
33963396
| array_flow.rb:1615:10:1615:13 | ...[...] [element 0] : | array_flow.rb:1615:10:1615:16 | ...[...] |
33973397
| array_flow.rb:1615:10:1615:13 | ...[...] [element 0] : | array_flow.rb:1615:10:1615:16 | ...[...] |
3398+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | array_flow.rb:1629:10:1629:10 | a [element 0] : |
3399+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | array_flow.rb:1629:10:1629:10 | a [element 0] : |
3400+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | array_flow.rb:1631:10:1631:10 | a [element 0] : |
3401+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | array_flow.rb:1631:10:1631:10 | a [element 0] : |
3402+
| array_flow.rb:1620:12:1620:24 | call to source : | array_flow.rb:1620:5:1620:5 | [post] a [element 0] : |
3403+
| array_flow.rb:1620:12:1620:24 | call to source : | array_flow.rb:1620:5:1620:5 | [post] a [element 0] : |
3404+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3405+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3406+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3407+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3408+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3409+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3410+
| array_flow.rb:1622:16:1622:28 | call to source : | array_flow.rb:1622:5:1622:5 | [post] a [element] : |
3411+
| array_flow.rb:1622:16:1622:28 | call to source : | array_flow.rb:1622:5:1622:5 | [post] a [element] : |
3412+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3413+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3414+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3415+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3416+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3417+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3418+
| array_flow.rb:1624:14:1624:26 | call to source : | array_flow.rb:1624:5:1624:5 | [post] a [element] : |
3419+
| array_flow.rb:1624:14:1624:26 | call to source : | array_flow.rb:1624:5:1624:5 | [post] a [element] : |
3420+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3421+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1627:10:1627:10 | a [element] : |
3422+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3423+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1629:10:1629:10 | a [element] : |
3424+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3425+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | array_flow.rb:1631:10:1631:10 | a [element] : |
3426+
| array_flow.rb:1626:16:1626:28 | call to source : | array_flow.rb:1626:5:1626:5 | [post] a [element] : |
3427+
| array_flow.rb:1626:16:1626:28 | call to source : | array_flow.rb:1626:5:1626:5 | [post] a [element] : |
3428+
| array_flow.rb:1627:10:1627:10 | a [element] : | array_flow.rb:1627:10:1627:13 | ...[...] |
3429+
| array_flow.rb:1627:10:1627:10 | a [element] : | array_flow.rb:1627:10:1627:13 | ...[...] |
3430+
| array_flow.rb:1629:10:1629:10 | a [element 0] : | array_flow.rb:1629:10:1629:17 | ...[...] |
3431+
| array_flow.rb:1629:10:1629:10 | a [element 0] : | array_flow.rb:1629:10:1629:17 | ...[...] |
3432+
| array_flow.rb:1629:10:1629:10 | a [element] : | array_flow.rb:1629:10:1629:17 | ...[...] |
3433+
| array_flow.rb:1629:10:1629:10 | a [element] : | array_flow.rb:1629:10:1629:17 | ...[...] |
3434+
| array_flow.rb:1631:10:1631:10 | a [element 0] : | array_flow.rb:1631:10:1631:15 | ...[...] |
3435+
| array_flow.rb:1631:10:1631:10 | a [element 0] : | array_flow.rb:1631:10:1631:15 | ...[...] |
3436+
| array_flow.rb:1631:10:1631:10 | a [element] : | array_flow.rb:1631:10:1631:15 | ...[...] |
3437+
| array_flow.rb:1631:10:1631:10 | a [element] : | array_flow.rb:1631:10:1631:15 | ...[...] |
33983438
nodes
33993439
| array_flow.rb:2:9:2:20 | * ... [element 0] : | semmle.label | * ... [element 0] : |
34003440
| array_flow.rb:2:9:2:20 | * ... [element 0] : | semmle.label | * ... [element 0] : |
@@ -7050,6 +7090,38 @@ nodes
70507090
| array_flow.rb:1615:10:1615:13 | ...[...] [element 0] : | semmle.label | ...[...] [element 0] : |
70517091
| array_flow.rb:1615:10:1615:16 | ...[...] | semmle.label | ...[...] |
70527092
| array_flow.rb:1615:10:1615:16 | ...[...] | semmle.label | ...[...] |
7093+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
7094+
| array_flow.rb:1620:5:1620:5 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
7095+
| array_flow.rb:1620:12:1620:24 | call to source : | semmle.label | call to source : |
7096+
| array_flow.rb:1620:12:1620:24 | call to source : | semmle.label | call to source : |
7097+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7098+
| array_flow.rb:1622:5:1622:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7099+
| array_flow.rb:1622:16:1622:28 | call to source : | semmle.label | call to source : |
7100+
| array_flow.rb:1622:16:1622:28 | call to source : | semmle.label | call to source : |
7101+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7102+
| array_flow.rb:1624:5:1624:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7103+
| array_flow.rb:1624:14:1624:26 | call to source : | semmle.label | call to source : |
7104+
| array_flow.rb:1624:14:1624:26 | call to source : | semmle.label | call to source : |
7105+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7106+
| array_flow.rb:1626:5:1626:5 | [post] a [element] : | semmle.label | [post] a [element] : |
7107+
| array_flow.rb:1626:16:1626:28 | call to source : | semmle.label | call to source : |
7108+
| array_flow.rb:1626:16:1626:28 | call to source : | semmle.label | call to source : |
7109+
| array_flow.rb:1627:10:1627:10 | a [element] : | semmle.label | a [element] : |
7110+
| array_flow.rb:1627:10:1627:10 | a [element] : | semmle.label | a [element] : |
7111+
| array_flow.rb:1627:10:1627:13 | ...[...] | semmle.label | ...[...] |
7112+
| array_flow.rb:1627:10:1627:13 | ...[...] | semmle.label | ...[...] |
7113+
| array_flow.rb:1629:10:1629:10 | a [element 0] : | semmle.label | a [element 0] : |
7114+
| array_flow.rb:1629:10:1629:10 | a [element 0] : | semmle.label | a [element 0] : |
7115+
| array_flow.rb:1629:10:1629:10 | a [element] : | semmle.label | a [element] : |
7116+
| array_flow.rb:1629:10:1629:10 | a [element] : | semmle.label | a [element] : |
7117+
| array_flow.rb:1629:10:1629:17 | ...[...] | semmle.label | ...[...] |
7118+
| array_flow.rb:1629:10:1629:17 | ...[...] | semmle.label | ...[...] |
7119+
| array_flow.rb:1631:10:1631:10 | a [element 0] : | semmle.label | a [element 0] : |
7120+
| array_flow.rb:1631:10:1631:10 | a [element 0] : | semmle.label | a [element 0] : |
7121+
| array_flow.rb:1631:10:1631:10 | a [element] : | semmle.label | a [element] : |
7122+
| array_flow.rb:1631:10:1631:10 | a [element] : | semmle.label | a [element] : |
7123+
| array_flow.rb:1631:10:1631:15 | ...[...] | semmle.label | ...[...] |
7124+
| array_flow.rb:1631:10:1631:15 | ...[...] | semmle.label | ...[...] |
70537125
subpaths
70547126
#select
70557127
| array_flow.rb:3:10:3:13 | ...[...] | array_flow.rb:2:10:2:20 | call to source : | array_flow.rb:3:10:3:13 | ...[...] | $@ | array_flow.rb:2:10:2:20 | call to source : | call to source : |
@@ -7737,3 +7809,14 @@ subpaths
77377809
| array_flow.rb:1614:10:1614:16 | ...[...] | array_flow.rb:1610:15:1610:27 | call to source : | array_flow.rb:1614:10:1614:16 | ...[...] | $@ | array_flow.rb:1610:15:1610:27 | call to source : | call to source : |
77387810
| array_flow.rb:1614:10:1614:16 | ...[...] | array_flow.rb:1613:15:1613:27 | call to source : | array_flow.rb:1614:10:1614:16 | ...[...] | $@ | array_flow.rb:1613:15:1613:27 | call to source : | call to source : |
77397811
| array_flow.rb:1615:10:1615:16 | ...[...] | array_flow.rb:1610:15:1610:27 | call to source : | array_flow.rb:1615:10:1615:16 | ...[...] | $@ | array_flow.rb:1610:15:1610:27 | call to source : | call to source : |
7812+
| array_flow.rb:1627:10:1627:13 | ...[...] | array_flow.rb:1622:16:1622:28 | call to source : | array_flow.rb:1627:10:1627:13 | ...[...] | $@ | array_flow.rb:1622:16:1622:28 | call to source : | call to source : |
7813+
| array_flow.rb:1627:10:1627:13 | ...[...] | array_flow.rb:1624:14:1624:26 | call to source : | array_flow.rb:1627:10:1627:13 | ...[...] | $@ | array_flow.rb:1624:14:1624:26 | call to source : | call to source : |
7814+
| array_flow.rb:1627:10:1627:13 | ...[...] | array_flow.rb:1626:16:1626:28 | call to source : | array_flow.rb:1627:10:1627:13 | ...[...] | $@ | array_flow.rb:1626:16:1626:28 | call to source : | call to source : |
7815+
| array_flow.rb:1629:10:1629:17 | ...[...] | array_flow.rb:1620:12:1620:24 | call to source : | array_flow.rb:1629:10:1629:17 | ...[...] | $@ | array_flow.rb:1620:12:1620:24 | call to source : | call to source : |
7816+
| array_flow.rb:1629:10:1629:17 | ...[...] | array_flow.rb:1622:16:1622:28 | call to source : | array_flow.rb:1629:10:1629:17 | ...[...] | $@ | array_flow.rb:1622:16:1622:28 | call to source : | call to source : |
7817+
| array_flow.rb:1629:10:1629:17 | ...[...] | array_flow.rb:1624:14:1624:26 | call to source : | array_flow.rb:1629:10:1629:17 | ...[...] | $@ | array_flow.rb:1624:14:1624:26 | call to source : | call to source : |
7818+
| array_flow.rb:1629:10:1629:17 | ...[...] | array_flow.rb:1626:16:1626:28 | call to source : | array_flow.rb:1629:10:1629:17 | ...[...] | $@ | array_flow.rb:1626:16:1626:28 | call to source : | call to source : |
7819+
| array_flow.rb:1631:10:1631:15 | ...[...] | array_flow.rb:1620:12:1620:24 | call to source : | array_flow.rb:1631:10:1631:15 | ...[...] | $@ | array_flow.rb:1620:12:1620:24 | call to source : | call to source : |
7820+
| array_flow.rb:1631:10:1631:15 | ...[...] | array_flow.rb:1622:16:1622:28 | call to source : | array_flow.rb:1631:10:1631:15 | ...[...] | $@ | array_flow.rb:1622:16:1622:28 | call to source : | call to source : |
7821+
| array_flow.rb:1631:10:1631:15 | ...[...] | array_flow.rb:1624:14:1624:26 | call to source : | array_flow.rb:1631:10:1631:15 | ...[...] | $@ | array_flow.rb:1624:14:1624:26 | call to source : | call to source : |
7822+
| array_flow.rb:1631:10:1631:15 | ...[...] | array_flow.rb:1626:16:1626:28 | call to source : | array_flow.rb:1631:10:1631:15 | ...[...] | $@ | array_flow.rb:1626:16:1626:28 | call to source : | call to source : |

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,3 +1614,19 @@ def m136(i)
16141614
sink(a[1][0]) # $ hasValueFlow=136.2 $ SPURIOUS hasValueFlow=136.1
16151615
sink(a[2][0]) # $ hasValueFlow=136.1
16161616
end
1617+
1618+
def m137
1619+
a = Array.new
1620+
a[0] = source(137.1)
1621+
# unknown store (we only track indices 0..10)
1622+
a[10000] = source(137.2)
1623+
# unknown store (we don't track floats)
1624+
a[1.0] = source(137.3)
1625+
# unknown store (we don't track complex numbers)
1626+
a[1.0+i] = source(137.4)
1627+
sink(a[2]) # $ hasValueFlow=137.2 $ hasValueFlow=137.3 $ hasValueFlow=137.4
1628+
# unknown read
1629+
sink(a[10001]) # $ hasValueFlow=137.1 $ hasValueFlow=137.2 $ hasValueFlow=137.3 $ hasValueFlow=137.4
1630+
# unknown read
1631+
sink(a[1.0]) # $ hasValueFlow=137.1 $ hasValueFlow=137.2 $ hasValueFlow=137.3 $ hasValueFlow=137.4
1632+
end

0 commit comments

Comments
 (0)