diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll index 37ef071d507..0748deaad55 100644 --- a/ql/src/codeql_ruby/ast/internal/AST.qll +++ b/ql/src/codeql_ruby/ast/internal/AST.qll @@ -94,10 +94,10 @@ private module Cached { not strictcount(int i | exists(g.getParent().(Generated::LeftAssignmentList).getChild(i))) = 1 } or TDivExpr(Generated::Binary g) { g instanceof @binary_slash } or - TDo(Generated::Do g) or + TDo(Generated::Do g) { exists(g.getChild(_)) } or TDoBlock(Generated::DoBlock g) { not g.getParent() instanceof Generated::Lambda } or TElementReference(Generated::ElementReference g) or - TElse(Generated::Else g) or + TElse(Generated::Else g) { exists(g.getChild(_)) } or TElsif(Generated::Elsif g) or TEmptyStmt(Generated::EmptyStatement g) or TEndBlock(Generated::EndBlock g) or @@ -198,7 +198,7 @@ private module Cached { TSubshellLiteral(Generated::Subshell g) or TSymbolArrayLiteral(Generated::SymbolArray g) or TTernaryIfExpr(Generated::Conditional g) or - TThen(Generated::Then g) or + TThen(Generated::Then g) { exists(g.getChild(_)) } or TTokenConstantAccess(Generated::Constant g) { // A tree-sitter `constant` token is a read of that constant in any context // where an identifier would be a vcall. diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index f6c5367d8ec..49affd6b7a6 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -643,10 +643,17 @@ module Trees { final override predicate first(AstNode first) { first(this.getCondition(), first) } + private AstNode getConditionSucc() { + result = this.getBody() + or + not exists(this.getBody()) and + result = this.getCondition() + } + final override predicate succ(AstNode pred, AstNode succ, Completion c) { last(this.getCondition(), pred, c) and this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue()) and - first(this.getBody(), succ) + first(this.getConditionSucc(), succ) or last(this.getBody(), pred, c) and first(this.getCondition(), succ) and diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected index 8b74d275522..e5d03ac8d5b 100644 --- a/ql/test/library-tests/ast/Ast.expected +++ b/ql/test/library-tests/ast/Ast.expected @@ -671,6 +671,18 @@ control/conditionals.rb: # 58| getBranch/getElse: [SubExpr] ... - ... # 58| getAnOperand/getLeftOperand: [LocalVariableAccess] e # 58| getAnOperand/getRightOperand: [IntegerLiteral] 2 +# 61| getStmt: [IfExpr] if ... +# 61| getCondition: [GTExpr] ... > ... +# 61| getAnOperand/getGreaterOperand/getLeftOperand: [LocalVariableAccess] a +# 61| getAnOperand/getLesserOperand/getRightOperand: [LocalVariableAccess] b +# 61| getBranch/getThen: [StmtSequence] c +# 62| getStmt: [LocalVariableAccess] c +# 67| getStmt: [IfExpr] if ... +# 67| getCondition: [GTExpr] ... > ... +# 67| getAnOperand/getGreaterOperand/getLeftOperand: [LocalVariableAccess] a +# 67| getAnOperand/getLesserOperand/getRightOperand: [LocalVariableAccess] b +# 68| getBranch/getElse: [StmtSequence] c +# 69| getStmt: [LocalVariableAccess] c constants/constants.rb: # 1| [Toplevel] constants.rb # 1| getStmt: [Module] ModuleA @@ -1262,6 +1274,10 @@ control/loops.rb: # 63| getCondition: [EqExpr] ... == ... # 63| getAnOperand/getLeftOperand: [LocalVariableAccess] x # 63| getAnOperand/getRightOperand: [IntegerLiteral] 0 +# 66| getStmt: [WhileExpr] while ... +# 66| getCondition: [LTExpr] ... < ... +# 66| getAnOperand/getLeftOperand/getLesserOperand: [LocalVariableAccess] x +# 66| getAnOperand/getGreaterOperand/getRightOperand: [LocalVariableAccess] y misc/misc.rb: # 1| [Toplevel] misc.rb # 1| getStmt: [AssignExpr] ... = ... diff --git a/ql/test/library-tests/ast/control/ConditionalExpr.expected b/ql/test/library-tests/ast/control/ConditionalExpr.expected index ce3c57f3471..ac2ac5ca057 100644 --- a/ql/test/library-tests/ast/control/ConditionalExpr.expected +++ b/ql/test/library-tests/ast/control/ConditionalExpr.expected @@ -18,6 +18,8 @@ conditionalExprs | conditionals.rb:55:1:55:18 | ... unless ... | UnlessModifierExpr | conditionals.rb:55:14:55:18 | ... < ... | conditionals.rb:55:1:55:5 | ... = ... | false | | conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr | conditionals.rb:58:5:58:9 | ... > ... | conditionals.rb:58:13:58:17 | ... + ... | true | | conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr | conditionals.rb:58:5:58:9 | ... > ... | conditionals.rb:58:21:58:25 | ... - ... | false | +| conditionals.rb:61:1:64:3 | if ... | IfExpr | conditionals.rb:61:4:61:8 | ... > ... | conditionals.rb:61:10:62:5 | c | true | +| conditionals.rb:67:1:70:3 | if ... | IfExpr | conditionals.rb:67:4:67:8 | ... > ... | conditionals.rb:68:1:69:5 | c | false | ifExprs | conditionals.rb:10:1:12:3 | if ... | IfExpr | conditionals.rb:10:4:10:8 | ... > ... | conditionals.rb:10:10:11:5 | c | (none) | false | | conditionals.rb:15:1:19:3 | if ... | IfExpr | conditionals.rb:15:4:15:9 | ... == ... | conditionals.rb:15:10:16:5 | c | d | false | @@ -26,6 +28,7 @@ ifExprs | conditionals.rb:26:1:29:5 | elsif ... | IfExpr | conditionals.rb:26:7:26:12 | ... == ... | conditionals.rb:26:14:27:5 | e | f | true | | conditionals.rb:33:1:37:3 | if ... | IfExpr | conditionals.rb:33:4:33:9 | ... == ... | conditionals.rb:33:10:34:5 | b | elsif ... | false | | conditionals.rb:35:1:36:5 | elsif ... | IfExpr | conditionals.rb:35:7:35:12 | ... == ... | conditionals.rb:35:13:36:5 | c | (none) | true | +| conditionals.rb:61:1:64:3 | if ... | IfExpr | conditionals.rb:61:4:61:8 | ... > ... | conditionals.rb:61:10:62:5 | c | (none) | false | unlessExprs | conditionals.rb:40:1:42:3 | unless ... | UnlessExpr | conditionals.rb:40:8:40:12 | ... > ... | conditionals.rb:40:14:41:5 | c | (none) | | conditionals.rb:45:1:49:3 | unless ... | UnlessExpr | conditionals.rb:45:8:45:13 | ... == ... | conditionals.rb:45:14:46:5 | c | d | diff --git a/ql/test/library-tests/ast/control/ControlExpr.expected b/ql/test/library-tests/ast/control/ControlExpr.expected index 3273ce0c6a4..9088fd6ed14 100644 --- a/ql/test/library-tests/ast/control/ControlExpr.expected +++ b/ql/test/library-tests/ast/control/ControlExpr.expected @@ -12,6 +12,8 @@ | conditionals.rb:52:1:52:14 | ... if ... | IfModifierExpr | | conditionals.rb:55:1:55:18 | ... unless ... | UnlessModifierExpr | | conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr | +| conditionals.rb:61:1:64:3 | if ... | IfExpr | +| conditionals.rb:67:1:70:3 | if ... | IfExpr | | loops.rb:9:1:12:3 | for ... in ... | ForExpr | | loops.rb:16:1:19:3 | for ... in ... | ForExpr | | loops.rb:22:1:25:3 | for ... in ... | ForExpr | @@ -22,3 +24,4 @@ | loops.rb:51:1:54:3 | until ... | UntilExpr | | loops.rb:57:1:60:3 | until ... | UntilExpr | | loops.rb:63:1:63:19 | ... until ... | UntilModifierExpr | +| loops.rb:66:1:67:3 | while ... | WhileExpr | diff --git a/ql/test/library-tests/ast/control/conditionals.rb b/ql/test/library-tests/ast/control/conditionals.rb index 946a2d6659f..85e008f5c1d 100644 --- a/ql/test/library-tests/ast/control/conditionals.rb +++ b/ql/test/library-tests/ast/control/conditionals.rb @@ -55,4 +55,16 @@ a = b if c > d a = b unless c < d # Ternary if expr -a = b > c ? d + 1 : e - 2 \ No newline at end of file +a = b > c ? d + 1 : e - 2 + +# If expr with empty else (treated as no else) +if a > b then + c +else +end + +# If expr with empty then (treated as no then) +if a > b then +else + c +end \ No newline at end of file diff --git a/ql/test/library-tests/ast/control/loops.rb b/ql/test/library-tests/ast/control/loops.rb index e6bead6c401..b00a61e348b 100644 --- a/ql/test/library-tests/ast/control/loops.rb +++ b/ql/test/library-tests/ast/control/loops.rb @@ -60,4 +60,8 @@ until x > y do end # Until-modified expression -x -= 1 until x == 0 \ No newline at end of file +x -= 1 until x == 0 + +# While loop with empty `do` block +while x < y do +end diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index b585b8ac4f0..52883ed1797 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -2330,7 +2330,7 @@ ifs.rb: #-----| -> true # 40| constant_condition -#-----| -> exit ifs.rb (normal) +#-----| -> empty_else # 40| exit constant_condition @@ -2346,6 +2346,39 @@ ifs.rb: # 41| true #-----| true -> [false] ! ... +# 46| enter empty_else +#-----| -> b + +# 46| empty_else +#-----| -> exit ifs.rb (normal) + +# 46| exit empty_else + +# 46| exit empty_else (normal) +#-----| -> exit empty_else + +# 46| b +#-----| -> b + +# 47| if ... +#-----| -> "done" + +# 47| b +#-----| true -> "true" +#-----| false -> if ... + +# 48| call to puts +#-----| -> if ... + +# 48| "true" +#-----| -> call to puts + +# 51| call to puts +#-----| -> exit empty_else (normal) + +# 51| "done" +#-----| -> call to puts + loops.rb: # 1| enter m1 #-----| -> x @@ -2503,7 +2536,7 @@ loops.rb: #-----| -> 1 # 24| m3 -#-----| -> exit loops.rb (normal) +#-----| -> m4 # 24| exit m3 @@ -2545,6 +2578,36 @@ loops.rb: # 26| x #-----| -> call to puts +# 30| enter m4 +#-----| -> x + +# 30| m4 +#-----| -> exit loops.rb (normal) + +# 30| exit m4 + +# 30| exit m4 (normal) +#-----| -> exit m4 + +# 30| x +#-----| -> y + +# 30| y +#-----| -> x + +# 31| while ... +#-----| -> exit m4 (normal) + +# 31| ... < ... +#-----| true -> x +#-----| false -> while ... + +# 31| x +#-----| -> y + +# 31| y +#-----| -> ... < ... + raise.rb: # 1| enter raise.rb #-----| -> ExceptionA diff --git a/ql/test/library-tests/controlflow/graph/ifs.rb b/ql/test/library-tests/controlflow/graph/ifs.rb index aa8ac8a342b..30a3f169a5d 100644 --- a/ql/test/library-tests/controlflow/graph/ifs.rb +++ b/ql/test/library-tests/controlflow/graph/ifs.rb @@ -42,3 +42,11 @@ def constant_condition() puts "Impossible" end end + +def empty_else b + if b then + puts "true" + else + end + puts "done" +end \ No newline at end of file diff --git a/ql/test/library-tests/controlflow/graph/loops.rb b/ql/test/library-tests/controlflow/graph/loops.rb index 01222db77e1..b3f1c60557a 100644 --- a/ql/test/library-tests/controlflow/graph/loops.rb +++ b/ql/test/library-tests/controlflow/graph/loops.rb @@ -25,4 +25,9 @@ def m3 [1,2,3].each do |x| puts x end -end \ No newline at end of file +end + +def m4(x, y) + while x < y do + end +end