Skip to content

Commit 6f7330b

Browse files
hvitvedhvitved
committed
Ruby: Do not attempt to track precise hash indices for floats and complex numbers
1 parent 4a39bc8 commit 6f7330b

File tree

5 files changed

+126
-17
lines changed

5 files changed

+126
-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: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,23 @@ private module Cached {
386386
lower)
387387
}
388388

389-
cached
390-
newtype TContent =
391-
TKnownElementContent(ConstantValue cv) {
389+
private predicate trackKnownValue(ConstantValue cv) {
390+
not cv.isFloat(_) and
391+
not cv.isComplex(_, _) and
392+
(
392393
not cv.isInt(_) or
393394
cv.getInt() in [0 .. 10]
394-
} or
395+
)
396+
}
397+
398+
cached
399+
newtype TContent =
400+
TKnownElementContent(ConstantValue cv) { trackKnownValue(cv) } or
395401
TUnknownElementContent() or
396-
TKnownPairValueContent(ConstantValue cv) or
402+
TKnownPairValueContent(ConstantValue cv) {
403+
trackKnownValue(cv) and
404+
not cv.isSymbol(_) // handled as a keyword argument
405+
} or
397406
TUnknownPairValueContent() or
398407
TFieldContent(string name) {
399408
name = any(InstanceVariable v).getName()
@@ -1026,18 +1035,11 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
10261035
// that is, where it is not a keyword argument.
10271036
node2.asExpr() =
10281037
any(CfgNodes::ExprNodes::PairCfgNode pair |
1029-
exists(CfgNodes::ExprCfgNode key |
1038+
exists(CfgNodes::ExprCfgNode key, ConstantValue cv |
10301039
key = pair.getKey() and
1031-
pair.getValue() = node1.asExpr()
1032-
|
1033-
exists(ConstantValue cv |
1034-
cv = key.getConstantValue() and
1035-
not cv.isSymbol(_) and // handled as a keyword argument
1036-
c.isSingleton(TKnownPairValueContent(cv))
1037-
)
1038-
or
1039-
not exists(key.getConstantValue()) and
1040-
c.isSingleton(TUnknownPairValueContent())
1040+
pair.getValue() = node1.asExpr() and
1041+
cv = key.getConstantValue() and
1042+
c.isSingleton(Content::getPairValueContent(cv))
10411043
)
10421044
)
10431045
or

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,14 @@ module Content {
314314
class UnknownPairValueContent extends PairValueContent, TUnknownPairValueContent {
315315
override string toString() { result = "pair" }
316316
}
317+
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+
}
317325
}
318326

319327
/**

‎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)