Skip to content

Commit 3613ceb

Browse files
authored
Merge pull request github#5535 from tausbn/python-prevent-bad-TCs
Approved by yoff
2 parents 7f16c52 + f17bbd9 commit 3613ceb

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1517,10 +1517,13 @@ predicate forReadStep(CfgNode nodeFrom, Content c, Node nodeTo) {
15171517
or
15181518
c instanceof SetElementContent
15191519
or
1520-
c instanceof TupleElementContent
1520+
c = small_tuple()
15211521
)
15221522
}
15231523

1524+
pragma[noinline]
1525+
TupleElementContent small_tuple() { result.getIndex() <= 7 }
1526+
15241527
/**
15251528
* Holds if `nodeTo` is a read of an attribute (corresponding to `c`) of the object in `nodeFrom`.
15261529
*

python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -467,14 +467,22 @@ class BarrierGuard extends GuardNode {
467467
}
468468
}
469469

470+
private predicate comes_from_cfgnode(Node node) {
471+
exists(CfgNode first, Node second |
472+
simpleLocalFlowStep(first, second) and
473+
simpleLocalFlowStep*(second, node)
474+
)
475+
}
476+
470477
/**
471478
* A data flow node that is a source of local flow. This includes things like
472479
* - Expressions
473480
* - Function parameters
474481
*/
475482
class LocalSourceNode extends Node {
483+
cached
476484
LocalSourceNode() {
477-
not simpleLocalFlowStep+(any(CfgNode n), this) and
485+
not comes_from_cfgnode(this) and
478486
not this instanceof ModuleVariableNode
479487
or
480488
this = any(ModuleVariableNode mvn).getARead()
@@ -522,15 +530,12 @@ private module Cached {
522530
* The slightly backwards parametering ordering is to force correct indexing.
523531
*/
524532
cached
525-
predicate hasLocalSource(Node sink, Node source) {
526-
// Declaring `source` to be a `SourceNode` currently causes a redundant check in the
527-
// recursive case, so instead we check it explicitly here.
528-
source = sink and
529-
source instanceof LocalSourceNode
533+
predicate hasLocalSource(Node sink, LocalSourceNode source) {
534+
source = sink
530535
or
531-
exists(Node mid |
532-
hasLocalSource(mid, source) and
533-
simpleLocalFlowStep(mid, sink)
536+
exists(Node second |
537+
simpleLocalFlowStep(source, second) and
538+
simpleLocalFlowStep*(second, sink)
534539
)
535540
}
536541

0 commit comments

Comments
 (0)