Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f0c761a

Browse filesBrowse files
hvitvedhvitved
committed
Ruby: Prevent synthetic splat matching for actual splats at same positions
1 parent 0264219 commit f0c761a
Copy full SHA for f0c761a

File tree

3 files changed

+53
-84
lines changed
Filter options

3 files changed

+53
-84
lines changed

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

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll
+41-40Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,11 @@ private module Cached {
563563
THashSplatArgumentPosition() or
564564
TSynthHashSplatArgumentPosition() or
565565
TSplatArgumentPosition(int pos) { exists(Call c | c.getArgument(pos) instanceof SplatExpr) } or
566-
TSynthSplatArgumentPosition(Boolean hasActualSplat) or
566+
TSynthSplatArgumentPosition(int actualSplatPos) {
567+
actualSplatPos = -1 // represents no actual splat
568+
or
569+
exists(Call c | c.getArgument(actualSplatPos) instanceof SplatExpr)
570+
} or
567571
TAnyArgumentPosition() or
568572
TAnyKeywordArgumentPosition()
569573

@@ -594,7 +598,11 @@ private module Cached {
594598
or
595599
exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
596600
} or
597-
TSynthSplatParameterPosition(Boolean hasActualSplat) or
601+
TSynthSplatParameterPosition(int actualSplatPos) {
602+
actualSplatPos = -1 // represents no actual splat
603+
or
604+
exists(Callable c | c.getParameter(actualSplatPos) instanceof SplatParameter)
605+
} or
598606
TAnyParameterPosition() or
599607
TAnyKeywordParameterPosition()
600608
}
@@ -1386,12 +1394,11 @@ class ParameterPosition extends TParameterPosition {
13861394
/**
13871395
* Holds if this position represents a synthetic splat parameter.
13881396
*
1389-
* `hasActualSplat` indicates whether the method that the parameter belongs
1390-
* to also has an actual splat parameter.
1397+
* `actualSplatPos` indicates the position of the (unique) actual splat
1398+
* parameter belonging to the same method, with `-1` representing no actual
1399+
* splat parameter.
13911400
*/
1392-
predicate isSynthSplat(boolean hasActualSplat) {
1393-
this = TSynthSplatParameterPosition(hasActualSplat)
1394-
}
1401+
predicate isSynthSplat(int actualSplatPos) { this = TSynthSplatParameterPosition(actualSplatPos) }
13951402

13961403
/**
13971404
* Holds if this position represents any parameter, except `self` parameters. This
@@ -1426,10 +1433,10 @@ class ParameterPosition extends TParameterPosition {
14261433
or
14271434
exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
14281435
or
1429-
exists(boolean hasActualSplat, string suffix |
1430-
this.isSynthSplat(hasActualSplat) and
1436+
exists(int actualSplatPos, string suffix |
1437+
this.isSynthSplat(actualSplatPos) and
14311438
result = "synthetic *" + suffix and
1432-
if hasActualSplat = true then suffix = " (with actual)" else suffix = ""
1439+
if actualSplatPos = -1 then suffix = "" else suffix = " (actual at " + actualSplatPos + ")"
14331440
)
14341441
}
14351442
}
@@ -1472,12 +1479,11 @@ class ArgumentPosition extends TArgumentPosition {
14721479
/**
14731480
* Holds if this position represents a synthetic splat argument.
14741481
*
1475-
* `hasActualSplat` indicates whether the call that the argument belongs
1476-
* to also has an actual splat argument.
1482+
* `actualSplatPos` indicates the position of the (unique) actual splat
1483+
* argument belonging to the same call, with `-1` representing no actual
1484+
* splat argument.
14771485
*/
1478-
predicate isSynthSplat(boolean hasActualSplat) {
1479-
this = TSynthSplatArgumentPosition(hasActualSplat)
1480-
}
1486+
predicate isSynthSplat(int actualSplatPos) { this = TSynthSplatArgumentPosition(actualSplatPos) }
14811487

14821488
/** Gets a textual representation of this position. */
14831489
string toString() {
@@ -1501,10 +1507,10 @@ class ArgumentPosition extends TArgumentPosition {
15011507
or
15021508
exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
15031509
or
1504-
exists(boolean hasActualSplat, string suffix |
1505-
this.isSynthSplat(hasActualSplat) and
1510+
exists(int actualSplatPos, string suffix |
1511+
this.isSynthSplat(actualSplatPos) and
15061512
result = "synthetic *" + suffix and
1507-
if hasActualSplat = true then suffix = " (with actual)" else suffix = ""
1513+
if actualSplatPos = -1 then suffix = "" else suffix = " (actual at " + actualSplatPos + ")"
15081514
)
15091515
}
15101516
}
@@ -1538,35 +1544,30 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
15381544
or
15391545
exists(string name | ppos.isKeyword(name) and apos.isKeyword(name))
15401546
or
1541-
ppos.isHashSplat() and
1542-
(apos.isHashSplat() or apos.isSynthHashSplat())
1543-
or
1544-
// no case for `apos.isSynthHashSplat() and ppos.isSynthHashSplat()`, since
1545-
// direct keyword matching is possible
1546-
ppos.isSynthHashSplat() and
1547-
apos.isHashSplat()
1547+
(ppos.isHashSplat() or ppos.isSynthHashSplat()) and
1548+
(apos.isHashSplat() or apos.isSynthHashSplat()) and
1549+
// prevent synthetic hash-splat parameters from matching synthetic hash-splat
1550+
// arguments when direct keyword matching is possible
1551+
not (ppos.isSynthHashSplat() and apos.isSynthHashSplat())
15481552
or
1549-
exists(int pos, boolean hasActualSplatParam, boolean hasActualSplatArg |
1553+
exists(int pos |
15501554
(
1551-
ppos.isSplat(pos) and
1552-
hasActualSplatParam = true // allow matching with synthetic splat argument
1555+
ppos.isSplat(pos)
15531556
or
1554-
ppos.isSynthSplat(hasActualSplatParam) and
1555-
pos = 0 and
1556-
// prevent synthetic splat parameters from matching synthetic splat arguments
1557-
// when direct positional matching is possible
1558-
(
1559-
hasActualSplatParam = true
1560-
or
1561-
hasActualSplatArg = true
1562-
)
1557+
ppos.isSynthSplat(_) and
1558+
pos = 0
15631559
) and
15641560
(
1565-
apos.isSplat(pos) and
1566-
hasActualSplatArg = true // allow matching with synthetic splat parameter
1561+
apos.isSplat(pos)
15671562
or
1568-
apos.isSynthSplat(hasActualSplatArg) and pos = 0
1563+
apos.isSynthSplat(_) and pos = 0
15691564
)
1565+
) and
1566+
// prevent synthetic splat parameters from matching synthetic splat arguments
1567+
// when direct positional matching is possible
1568+
not exists(int actualSplatPos |
1569+
ppos.isSynthSplat(actualSplatPos) and
1570+
apos.isSynthSplat(actualSplatPos)
15701571
)
15711572
or
15721573
ppos.isAny() and argumentPositionIsNotSelf(apos)

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

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+12-10Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,11 +1204,11 @@ private module ParameterNodes {
12041204

12051205
final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
12061206
c = callable and
1207-
exists(boolean hasActualSplat |
1208-
pos.isSynthSplat(hasActualSplat) and
1209-
if exists(TSynthSplatParameterShiftNode(c, _, _))
1210-
then hasActualSplat = true
1211-
else hasActualSplat = false
1207+
exists(int actualSplat | pos.isSynthSplat(actualSplat) |
1208+
exists(TSynthSplatParameterShiftNode(c, actualSplat, _))
1209+
or
1210+
not exists(TSynthSplatParameterShiftNode(c, _, _)) and
1211+
actualSplat = -1
12121212
)
12131213
}
12141214

@@ -1488,11 +1488,13 @@ module ArgumentNodes {
14881488

14891489
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
14901490
call = call_ and
1491-
exists(boolean hasActualSplat |
1492-
pos.isSynthSplat(hasActualSplat) and
1493-
if any(SynthSplatArgumentShiftNode shift).storeInto(this, _)
1494-
then hasActualSplat = true
1495-
else hasActualSplat = false
1491+
exists(int actualSplat | pos.isSynthSplat(actualSplat) |
1492+
any(SynthSplatArgumentShiftNode shift |
1493+
shift = TSynthSplatArgumentShiftNode(_, actualSplat, _)
1494+
).storeInto(this, _)
1495+
or
1496+
not any(SynthSplatArgumentShiftNode shift).storeInto(this, _) and
1497+
actualSplat = -1
14961498
)
14971499
}
14981500

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.