Skip to content

Commit 800e183

Browse files
committed
Add != to StringConstCompare
This means we treat != comparisons against strings as taint tracking guards: if foo != "A" foo # still tainted else foo # not tainted, because we know foo == "A" end
1 parent 8f36b0d commit 800e183

File tree

3 files changed

+23
-9
lines changed

3 files changed

+23
-9
lines changed

ql/lib/codeql/ruby/dataflow/BarrierGuards.qll

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,26 @@ private import codeql.ruby.CFG
2020
class StringConstCompare extends DataFlow::BarrierGuard,
2121
CfgNodes::ExprNodes::ComparisonOperationCfgNode {
2222
private CfgNode checkedNode;
23+
// The value of the condition that results in the node being validated.
24+
private boolean checkedBranch;
2325

2426
StringConstCompare() {
2527
exists(CfgNodes::ExprNodes::StringLiteralCfgNode strLitNode |
26-
this.getExpr() instanceof EqExpr or
27-
this.getExpr() instanceof CaseEqExpr
28+
this.getExpr() instanceof EqExpr and checkedBranch = true
29+
or
30+
this.getExpr() instanceof CaseEqExpr and checkedBranch = true
31+
or
32+
this.getExpr() instanceof NEExpr and checkedBranch = false
2833
|
2934
this.getLeftOperand() = strLitNode and this.getRightOperand() = checkedNode
3035
or
3136
this.getLeftOperand() = checkedNode and this.getRightOperand() = strLitNode
3237
)
3338
}
3439

35-
override predicate checks(CfgNode expr, boolean branch) { expr = checkedNode and branch = true }
40+
override predicate checks(CfgNode expr, boolean branch) {
41+
expr = checkedNode and branch = checkedBranch
42+
}
3643
}
3744

3845
/**
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1-
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:15:4:17 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
2-
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:15:10:17 | foo | barrier-guards.rb:9:21:9:23 | foo | true |
1+
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:13:4:15 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
2+
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:13:10:15 | foo | barrier-guards.rb:9:21:9:23 | foo | true |
3+
| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:18:14:18:16 | foo | barrier-guards.rb:15:4:15:6 | foo | false |

ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
foo = "foo"
22

33
if foo == "foo"
4-
do_true_1 foo
4+
do_true foo
55
else
6-
do_false_1 foo
6+
do_false foo
77
end
88

99
if ["foo"].include?(foo)
10-
do_true_2 foo
10+
do_true foo
1111
else
12-
do_false_2 foo
12+
do_false foo
13+
end
14+
15+
if foo != "foo"
16+
do_true foo
17+
else
18+
do_false foo
1319
end
1420

1521
do_default foo

0 commit comments

Comments
 (0)