Skip to content

Commit 951d545

Browse files
hvitvedhvitved
committed
refactor2
1 parent 29ac38c commit 951d545

File tree

3 files changed

+70
-67
lines changed

3 files changed

+70
-67
lines changed

‎shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

Lines changed: 58 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,10 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
10211021
callEdgeReturn(call, c, _, _, _, _, _)
10221022
}
10231023

1024+
predicate storeMayReachRead(NodeEx storeSource, Content c, NodeEx readTarget) { none() }
1025+
1026+
predicate providesStoreReadMatching() { none() }
1027+
10241028
additional predicate stats(
10251029
boolean fwd, int nodes, int fields, int conscand, int states, int tuples, int calledges
10261030
) {
@@ -1320,6 +1324,10 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
13201324
predicate relevantCallEdgeIn(DataFlowCall call, DataFlowCallable c);
13211325

13221326
predicate relevantCallEdgeOut(DataFlowCall call, DataFlowCallable c);
1327+
1328+
predicate storeMayReachRead(NodeEx storeSource, Content c, NodeEx readTarget);
1329+
1330+
default predicate providesStoreReadMatching() { any() }
13231331
}
13241332

13251333
private module MkStage<StageSig PrevStage> {
@@ -1425,8 +1433,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
14251433
predicate typecheckStore(Typ typ, DataFlowType contentType);
14261434

14271435
default predicate enableTypeFlow() { any() }
1428-
1429-
default predicate enableStoreReadMatching() { any() }
14301436
}
14311437

14321438
module Stage<StageParam Param> implements StageSig {
@@ -1457,43 +1463,49 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
14571463

14581464
private class NodeExAlias = NodeEx;
14591465

1466+
final private class ApFinal = Ap;
1467+
14601468
private module StoreReadMatchingInput implements StoreReadMatchingInputSig {
14611469
class NodeEx = NodeExAlias;
14621470

1463-
final private class ApApproxFinal = PrevStage::Ap;
1464-
1465-
class Ap extends ApApproxFinal {
1466-
Content getHead() { result = PrevStage::getAHeadContent(this) }
1471+
class Ap extends ApFinal {
1472+
Content getHead() { result = getAHeadContent(this) }
14671473
}
14681474

1469-
predicate revFlow(NodeEx node, Ap ap) {
1470-
enableStoreReadMatching() and PrevStage::revFlowAp(node, ap)
1471-
}
1475+
predicate nodeApRange(NodeEx node, Ap ap) { revFlowAp(node, ap) }
14721476

1473-
predicate localValueStep(NodeEx node1, NodeEx node2) { localValueStep(node1, node2, _) }
1477+
predicate localValueStep(NodeEx node1, NodeEx node2) {
1478+
exists(FlowState state, Ap ap, ApOption returnAp |
1479+
revFlow(node1, pragma[only_bind_into](state), _, pragma[only_bind_into](returnAp),
1480+
pragma[only_bind_into](ap)) and
1481+
revFlow(node2, pragma[only_bind_into](state), _, pragma[only_bind_into](returnAp),
1482+
pragma[only_bind_into](ap)) and
1483+
localValueStep(node1, node2, _)
1484+
)
1485+
}
14741486

14751487
predicate jumpValueStep(NodeEx node1, NodeEx node2) { jumpStepEx(node1, node2) }
14761488

1477-
predicate callEdgeArgParam(NodeEx arg, NodeEx param, Ap ap) {
1478-
PrevStage::callEdgeArgParam(_, _, arg, param, true, ap)
1489+
predicate callEdgeArgParam(NodeEx arg, NodeEx param) {
1490+
callEdgeArgParam(_, _, arg, param, true, _)
14791491
}
14801492

1481-
predicate callEdgeReturn(NodeEx ret, NodeEx out, Ap ap) {
1482-
PrevStage::callEdgeReturn(_, _, ret, _, out, true, ap)
1493+
predicate callEdgeReturn(NodeEx ret, NodeEx out) {
1494+
callEdgeReturn(_, _, ret, _, out, true, _)
14831495
}
14841496

14851497
predicate readContentStep(NodeEx node1, Content c, NodeEx node2) {
1486-
PrevStage::readStepCand(node1, c, node2)
1498+
readStepCand0(node1, c, node2)
14871499
}
14881500

14891501
predicate storeContentStep(NodeEx node1, Content c, NodeEx node2) {
1490-
PrevStage::storeStepCand(node1, _, c, node2, _, _)
1502+
storeStepCand0(node1, _, c, node2, _, _)
14911503
}
14921504

14931505
int accessPathConfigLimit() { result = Config::accessPathLimit() }
14941506
}
14951507

1496-
private import StoreReadMatching<StoreReadMatchingInput>
1508+
import StoreReadMatching<StoreReadMatchingInput>
14971509

14981510
/**
14991511
* Holds if `node` is reachable with access path `ap` from a source.
@@ -1529,7 +1541,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
15291541
private predicate storeMayReachReadInlineLate(
15301542
NodeEx storeSource, Content c, NodeEx readTarget
15311543
) {
1532-
storeMayReachRead(storeSource, c, readTarget)
1544+
PrevStage::storeMayReachRead(storeSource, c, readTarget)
15331545
}
15341546

15351547
bindingset[node1, state1]
@@ -1656,8 +1668,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
16561668
fwdFlow(node1, state, cc, summaryCtx, argT, argAp, t1, ap1, apa1) and
16571669
PrevStage::storeStepCand(node1, apa1, c, node2, contentType, containerType) and
16581670
t2 = getTyp(containerType) and
1659-
typecheckStore(t1, contentType) and
1660-
if enableStoreReadMatching() then storeMayReachRead(node1, c, _) else any()
1671+
typecheckStore(t1, contentType)
16611672
)
16621673
}
16631674

@@ -1666,13 +1677,14 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
16661677
* store of `c` on a container of type `t2` resulting in access path
16671678
* `cons`. `storeSource` is a relevant store source.
16681679
*
1669-
* This predicate is only evaluated when `enableStoreReadMatching()` holds.
1680+
* This predicate is only evaluated when `PrevStage::providesStoreReadMatching()`
1681+
* holds.
16701682
*/
16711683
pragma[nomagic]
16721684
private predicate fwdFlowConsCandStoreReadMatchingEnabled(
16731685
NodeEx storeSource, Typ t2, Ap cons, Content c, Typ t1, Ap tail
16741686
) {
1675-
enableStoreReadMatching() and
1687+
PrevStage::providesStoreReadMatching() and
16761688
fwdFlowStore(storeSource, t1, tail, c, t2, _, _, _, _, _, _) and
16771689
cons = apCons(c, t1, tail)
16781690
or
@@ -1687,14 +1699,14 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
16871699
* store of `c` on a container of type `t2` resulting in access path
16881700
* `cons`.
16891701
*
1690-
* This predicate is only evaluated when `enableStoreReadMatching()`
1702+
* This predicate is only evaluated when `PrevStage::providesStoreReadMatching()`
16911703
* doesn't hold.
16921704
*/
16931705
pragma[nomagic]
16941706
private predicate fwdFlowConsCandStoreReadMatchingDisabled(
16951707
Typ t2, Ap cons, Content c, Typ t1, Ap tail
16961708
) {
1697-
not enableStoreReadMatching() and
1709+
not PrevStage::providesStoreReadMatching() and
16981710
fwdFlowStore(_, t1, tail, c, t2, _, _, _, _, _, _) and
16991711
cons = apCons(c, t1, tail)
17001712
or
@@ -1707,8 +1719,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
17071719
pragma[nomagic]
17081720
private predicate readStepCand(NodeEx node1, ApHeadContent apc, Content c, NodeEx node2) {
17091721
PrevStage::readStepCand(node1, c, node2) and
1710-
apc = projectToHeadContent(c) and
1711-
if enableStoreReadMatching() then storeMayReachRead(_, c, node2) else any()
1722+
apc = projectToHeadContent(c)
17121723
}
17131724

17141725
bindingset[node1, apc]
@@ -2360,7 +2371,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
23602371
*/
23612372
pragma[nomagic]
23622373
private predicate revFlowConsCandStoreReadMatchingDisabled(Ap cons, Content c, Ap tail) {
2363-
not enableStoreReadMatching() and
2374+
not PrevStage::providesStoreReadMatching() and
23642375
revFlowConsCand(_, cons, c, tail)
23652376
}
23662377

@@ -2481,7 +2492,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
24812492
}
24822493

24832494
pragma[nomagic]
2484-
predicate storeStepCand(
2495+
private predicate storeStepCand0(
24852496
NodeEx node1, Ap ap1, Content c, NodeEx node2, DataFlowType contentType,
24862497
DataFlowType containerType
24872498
) {
@@ -2493,7 +2504,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
24932504
}
24942505

24952506
pragma[nomagic]
2496-
predicate readStepCand(NodeEx node1, Content c, NodeEx node2) {
2507+
private predicate readStepCand0(NodeEx node1, Content c, NodeEx node2) {
24972508
exists(Ap ap1, Ap ap2 |
24982509
revFlow(node2, _, _, _, pragma[only_bind_into](ap2)) and
24992510
readStepFwd(node1, ap1, c, node2, ap2) and
@@ -2514,12 +2525,9 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
25142525
private predicate fwdConsCand(Content c, Typ t, Ap ap) { storeStepFwd(_, t, ap, c, _, _) }
25152526

25162527
private predicate revConsCand(NodeEx readTarget, Content c, Typ t, Ap ap) {
2517-
exists(NodeEx storeSource, Ap ap2 |
2518-
revFlowStore(ap2, c, ap, t, storeSource, _, _, _, _) and
2519-
revFlowConsCand(readTarget, ap2, c, ap) and
2520-
if enableStoreReadMatching()
2521-
then storeMayReachRead(storeSource, c, readTarget)
2522-
else any()
2528+
exists(Ap ap2 |
2529+
revFlowStore(ap2, c, ap, t, _, _, _, _, _) and
2530+
revFlowConsCand(readTarget, ap2, c, ap)
25232531
)
25242532
}
25252533

@@ -2622,6 +2630,21 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
26222630
callEdgeReturn(call, c, _, _, _, _, _)
26232631
}
26242632

2633+
pragma[nomagic]
2634+
predicate storeStepCand(
2635+
NodeEx node1, Ap ap1, Content c, NodeEx node2, DataFlowType contentType,
2636+
DataFlowType containerType
2637+
) {
2638+
storeStepCand0(node1, ap1, c, node2, contentType, containerType) and
2639+
storeMayReachRead(node1, c, _)
2640+
}
2641+
2642+
pragma[nomagic]
2643+
predicate readStepCand(NodeEx node1, Content c, NodeEx node2) {
2644+
readStepCand0(node1, c, node2) and
2645+
storeMayReachRead(_, c, node2)
2646+
}
2647+
26252648
additional predicate stats(
26262649
boolean fwd, int nodes, int fields, int conscand, int states, int tuples, int calledges,
26272650
int tfnodes, int tftuples
@@ -2797,8 +2820,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
27972820
predicate typecheckStore(Typ typ, DataFlowType contentType) { any() }
27982821

27992822
predicate enableTypeFlow() { none() }
2800-
2801-
predicate enableStoreReadMatching() { none() }
28022823
}
28032824

28042825
private module Stage2 = MkStage<Stage1>::Stage<Stage2Param>;

‎shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2363,15 +2363,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
23632363
Content getHead();
23642364
}
23652365

2366-
predicate revFlow(NodeEx node, Ap ap);
2366+
predicate nodeApRange(NodeEx node, Ap ap);
23672367

23682368
predicate localValueStep(NodeEx node1, NodeEx node2);
23692369

23702370
predicate jumpValueStep(NodeEx node1, NodeEx node2);
23712371

2372-
predicate callEdgeArgParam(NodeEx arg, NodeEx param, Ap ap);
2372+
predicate callEdgeArgParam(NodeEx arg, NodeEx param);
23732373

2374-
predicate callEdgeReturn(NodeEx ret, NodeEx out, Ap ap);
2374+
predicate callEdgeReturn(NodeEx ret, NodeEx out);
23752375

23762376
predicate readContentStep(NodeEx node1, Content c, NodeEx node2);
23772377

@@ -2410,16 +2410,16 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
24102410
pragma[nomagic]
24112411
private predicate valueStep(NodeEx node1, NodeEx node2) {
24122412
exists(ApCons ap |
2413-
revFlow(node1, pragma[only_bind_into](ap)) and
2414-
revFlow(node2, pragma[only_bind_into](ap))
2413+
nodeApRange(node1, pragma[only_bind_into](ap)) and
2414+
nodeApRange(node2, pragma[only_bind_into](ap))
24152415
|
24162416
localValueStep(node1, node2)
24172417
or
24182418
jumpValueStep(node1, node2)
24192419
or
2420-
callEdgeArgParam(node1, node2, ap)
2420+
callEdgeArgParam(node1, node2)
24212421
or
2422-
callEdgeReturn(node1, node2, ap)
2422+
callEdgeReturn(node1, node2)
24232423
)
24242424
}
24252425

@@ -2448,7 +2448,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
24482448
private newtype TNodeOrContent =
24492449
TNodeOrContentNode(NodeEx n, ApCons ap, UsesPrevDeltaInfo usesPrevDelta) {
24502450
enabled() and
2451-
revFlow(n, ap)
2451+
nodeApRange(n, ap)
24522452
} or
24532453
TNodeOrContentContentStart(Content c) { enabled() and storeContentStep(_, c, _) } or
24542454
TNodeOrContentContentEnd(Content c) { enabled() and readContentStep(_, c, _) }
@@ -2530,6 +2530,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
25302530
)
25312531
}
25322532

2533+
pragma[nomagic]
25332534
private predicate isStoreTarget0(NodeOrContent node, Content c) {
25342535
exists(Ap ap, UsesPrevDeltaInfo usesPrevDelta |
25352536
contentIsReadAndStored(c) and
@@ -2541,6 +2542,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
25412542

25422543
private predicate isStoreTarget(NodeOrContent node) { isStoreTarget0(node, _) }
25432544

2545+
pragma[nomagic]
25442546
private predicate isReadSource0(NodeOrContent node, Content c) {
25452547
exists(Ap ap, UsesPrevDeltaInfo usesPrevDelta |
25462548
contentIsReadAndStored(c) and
@@ -2584,15 +2586,9 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
25842586
)
25852587
}
25862588

2587-
private predicate isStoreTargetPruned(NodeOrContent node) {
2588-
isStoreTarget(node) and
2589-
storeMayReachARead(node, _)
2590-
}
2589+
private predicate isStoreTargetPruned(NodeOrContent node) { storeMayReachARead(node, _) }
25912590

2592-
private predicate isReadSourcePruned(NodeOrContent node) {
2593-
isReadSource(node) and
2594-
aStoreMayReachRead(node, _)
2595-
}
2591+
private predicate isReadSourcePruned(NodeOrContent node) { aStoreMayReachRead(node, _) }
25962592

25972593
private predicate storeMayReachReadTc(NodeOrContent node1, NodeOrContent node2) =
25982594
doublyBoundedFastTC(step/2, isStoreTargetPruned/1, isReadSourcePruned/1)(node1, node2)

‎swift/ql/test/query-tests/Security/CWE-078/CommandInjection.expected

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ edges
4444
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:131:36:131:36 | userControlledString | provenance | |
4545
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:132:21:132:21 | userControlledString | provenance | |
4646
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:133:22:133:22 | userControlledString | provenance | |
47-
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:134:24:134:24 | userControlledString | provenance | |
4847
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:144:42:144:42 | userControlledString | provenance | |
4948
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:145:75:145:75 | userControlledString | provenance | |
5049
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:148:35:148:35 | userControlledString | provenance | |
@@ -65,7 +64,6 @@ edges
6564
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:131:36:131:36 | userControlledString | provenance | |
6665
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:132:21:132:21 | userControlledString | provenance | |
6766
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:133:22:133:22 | userControlledString | provenance | |
68-
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:134:24:134:24 | userControlledString | provenance | |
6967
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:144:42:144:42 | userControlledString | provenance | |
7068
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:145:75:145:75 | userControlledString | provenance | |
7169
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:148:35:148:35 | userControlledString | provenance | |
@@ -110,14 +108,6 @@ edges
110108
| CommandInjection.swift:132:21:132:21 | userControlledString | CommandInjection.swift:132:20:132:41 | [...] [Collection element] | provenance | |
111109
| CommandInjection.swift:133:21:133:42 | [...] [Collection element] | CommandInjection.swift:93:20:93:40 | arguments [Collection element] | provenance | |
112110
| CommandInjection.swift:133:22:133:22 | userControlledString | CommandInjection.swift:133:21:133:42 | [...] [Collection element] | provenance | |
113-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:144:42:144:42 | userControlledString | provenance | |
114-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:145:75:145:75 | userControlledString | provenance | |
115-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:148:35:148:35 | userControlledString | provenance | |
116-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:149:70:149:70 | userControlledString | provenance | |
117-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:154:53:154:53 | userControlledString | provenance | |
118-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:157:52:157:52 | userControlledString | provenance | |
119-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:158:33:158:33 | userControlledString | provenance | |
120-
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:160:57:160:57 | userControlledString | provenance | |
121111
| CommandInjection.swift:145:67:145:95 | [...] [Collection element] | CommandInjection.swift:145:67:145:95 | [...] | provenance | |
122112
| CommandInjection.swift:145:75:145:75 | userControlledString | CommandInjection.swift:145:67:145:95 | [...] [Collection element] | provenance | |
123113
| CommandInjection.swift:148:23:148:55 | call to URL.init(string:) [some:0] | CommandInjection.swift:148:23:148:56 | ...! | provenance | |
@@ -158,11 +148,8 @@ edges
158148
| CommandInjection.swift:186:19:186:19 | userControlledString | CommandInjection.swift:186:18:186:39 | [...] [Collection element] | provenance | |
159149
| CommandInjection.swift:188:3:188:3 | [post] getter for .p1 [arguments, Collection element] | CommandInjection.swift:188:3:188:3 | [post] getter for .p1 | provenance | |
160150
| CommandInjection.swift:188:18:188:18 | tainted1 [Collection element] | CommandInjection.swift:188:3:188:3 | [post] getter for .p1 [arguments, Collection element] | provenance | |
161-
| CommandInjection.swift:188:18:188:18 | tainted1 [Collection element] | CommandInjection.swift:189:19:189:19 | tainted1 [Collection element] | provenance | |
162-
| CommandInjection.swift:188:18:188:18 | tainted1 [Collection element] | CommandInjection.swift:190:18:190:18 | tainted1 [Collection element] | provenance | |
163151
| CommandInjection.swift:189:3:189:5 | [post] ...! [arguments, Collection element] | CommandInjection.swift:189:3:189:5 | [post] ...! | provenance | |
164152
| CommandInjection.swift:189:19:189:19 | tainted1 [Collection element] | CommandInjection.swift:189:3:189:5 | [post] ...! [arguments, Collection element] | provenance | |
165-
| CommandInjection.swift:189:19:189:19 | tainted1 [Collection element] | CommandInjection.swift:190:18:190:18 | tainted1 [Collection element] | provenance | |
166153
| CommandInjection.swift:190:3:190:3 | [post] ...! [arguments, Collection element] | CommandInjection.swift:190:3:190:3 | [post] ...! | provenance | |
167154
| CommandInjection.swift:190:18:190:18 | tainted1 [Collection element] | CommandInjection.swift:190:3:190:3 | [post] ...! [arguments, Collection element] | provenance | |
168155
| CommandInjection.swift:192:30:192:51 | [...] [Collection element] | CommandInjection.swift:194:18:194:18 | tainted2 [Collection element] | provenance | |
@@ -259,7 +246,6 @@ nodes
259246
| CommandInjection.swift:132:21:132:21 | userControlledString | semmle.label | userControlledString |
260247
| CommandInjection.swift:133:21:133:42 | [...] [Collection element] | semmle.label | [...] [Collection element] |
261248
| CommandInjection.swift:133:22:133:22 | userControlledString | semmle.label | userControlledString |
262-
| CommandInjection.swift:134:24:134:24 | userControlledString | semmle.label | userControlledString |
263249
| CommandInjection.swift:144:42:144:42 | userControlledString | semmle.label | userControlledString |
264250
| CommandInjection.swift:145:67:145:95 | [...] | semmle.label | [...] |
265251
| CommandInjection.swift:145:67:145:95 | [...] [Collection element] | semmle.label | [...] [Collection element] |

0 commit comments

Comments
 (0)