Skip to content

Commit f1b63c4

Browse files
committed
Ruby: Fix in clause barrier guard
1 parent 0ab88c2 commit f1b63c4

File tree

3 files changed

+38
-30
lines changed

3 files changed

+38
-30
lines changed

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

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -159,42 +159,44 @@ private predicate stringConstCaseCompare(
159159
branch = true and
160160
exists(CfgNodes::ExprNodes::CaseExprCfgNode case |
161161
case.getValue() = testedNode and
162-
exists(CfgNodes::ExprNodes::WhenClauseCfgNode branchNode |
163-
branchNode = case.getBranch(_) and
164-
guard = branchNode.getPattern(_) and
165-
// For simplicity, consider patterns that contain only string literals or arrays of string literals
166-
forall(ExprCfgNode pattern | pattern = branchNode.getPattern(_) |
167-
// when "foo"
168-
// when "foo", "bar"
169-
pattern instanceof ExprNodes::StringLiteralCfgNode
170-
or
171-
exists(CfgNodes::ExprNodes::SplatExprCfgNode splat | splat = pattern |
172-
// when *["foo", "bar"]
173-
forex(ExprCfgNode elem |
174-
elem = splat.getOperand().(ExprNodes::ArrayLiteralCfgNode).getAnArgument()
175-
|
176-
elem instanceof ExprNodes::StringLiteralCfgNode
177-
)
162+
(
163+
exists(CfgNodes::ExprNodes::WhenClauseCfgNode branchNode |
164+
branchNode = case.getBranch(_) and
165+
guard = branchNode.getPattern(_) and
166+
// For simplicity, consider patterns that contain only string literals or arrays of string literals
167+
forall(ExprCfgNode pattern | pattern = branchNode.getPattern(_) |
168+
// when "foo"
169+
// when "foo", "bar"
170+
pattern instanceof ExprNodes::StringLiteralCfgNode
178171
or
179-
// when *some_var
180-
// when *SOME_CONST
181-
exists(ExprNodes::ArrayLiteralCfgNode arr |
182-
isArrayConstant(splat.getOperand(), arr) and
183-
forall(ExprCfgNode elem | elem = arr.getAnArgument() |
172+
exists(CfgNodes::ExprNodes::SplatExprCfgNode splat | splat = pattern |
173+
// when *["foo", "bar"]
174+
forex(ExprCfgNode elem |
175+
elem = splat.getOperand().(ExprNodes::ArrayLiteralCfgNode).getAnArgument()
176+
|
184177
elem instanceof ExprNodes::StringLiteralCfgNode
185178
)
179+
or
180+
// when *some_var
181+
// when *SOME_CONST
182+
exists(ExprNodes::ArrayLiteralCfgNode arr |
183+
isArrayConstant(splat.getOperand(), arr) and
184+
forall(ExprCfgNode elem | elem = arr.getAnArgument() |
185+
elem instanceof ExprNodes::StringLiteralCfgNode
186+
)
187+
)
186188
)
187189
)
188190
)
189-
)
190-
or
191-
// in "foo"
192-
exists(
193-
CfgNodes::ExprNodes::InClauseCfgNode branchNode, ExprNodes::StringLiteralCfgNode pattern
194-
|
195-
branchNode = case.getBranch(_) and
196-
pattern = branchNode.getPattern() and
197-
guard = pattern
191+
or
192+
// in "foo"
193+
exists(
194+
CfgNodes::ExprNodes::InClauseCfgNode branchNode, ExprNodes::StringLiteralCfgNode pattern
195+
|
196+
branchNode = case.getBranch(_) and
197+
pattern = branchNode.getPattern() and
198+
guard = pattern
199+
)
198200
)
199201
)
200202
}

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,4 @@ controls
123123
| barrier-guards.rb:207:4:207:8 | "foo" | barrier-guards.rb:209:1:210:7 | in ... then ... | no-match |
124124
| barrier-guards.rb:207:4:207:8 | "foo" | barrier-guards.rb:210:5:210:7 | foo | no-match |
125125
| barrier-guards.rb:209:4:209:4 | x | barrier-guards.rb:210:5:210:7 | foo | match |
126+
| barrier-guards.rb:214:4:214:8 | "foo" | barrier-guards.rb:215:5:215:7 | foo | match |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,8 @@
209209
in x
210210
foo
211211
end
212+
213+
case bar
214+
in "foo"
215+
foo
216+
end

0 commit comments

Comments
 (0)