From 6d2d9654b53a59db7c45333896c6c0b52e2bce4a Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 4 Apr 2024 13:01:00 +0200 Subject: [PATCH 1/2] Ruby: Add CFG test --- .../controlflow/graph/Cfg.expected | 105 +++++++++++++++--- .../controlflow/graph/Nodes.expected | 4 + .../library-tests/controlflow/graph/ifs.rb | 2 +- .../library-tests/controlflow/graph/raise.rb | 12 ++ 4 files changed, 105 insertions(+), 18 deletions(-) diff --git a/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected b/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected index e3b0e01adfe7..2571a9dfeeb4 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -2397,18 +2397,9 @@ cfg.rb: # 90| ... #-----| -> $global -# 90| ... = ... -#-----| -> if ... - # 90| ... = ... #-----| -> x -# 90| [false] ! ... -#-----| false -> if ... - -# 90| [true] ! ... -#-----| true -> x - # 90| __synth__0__1 #-----| -> x @@ -2418,10 +2409,6 @@ cfg.rb: # 90| call to each #-----| -> ... -# 90| defined? ... -#-----| false -> [true] ! ... -#-----| true -> [false] ! ... - # 90| enter { ... } #-----| -> __synth__0__1 @@ -2430,6 +2417,22 @@ cfg.rb: # 90| exit { ... } (normal) #-----| -> exit { ... } +# 90| { ... } +#-----| -> call to each + +# 90| ... = ... +#-----| -> if ... + +# 90| [false] ! ... +#-----| false -> if ... + +# 90| [true] ! ... +#-----| true -> x + +# 90| defined? ... +#-----| false -> [true] ! ... +#-----| true -> [false] ! ... + # 90| if ... #-----| -> Array @@ -2442,9 +2445,6 @@ cfg.rb: # 90| x #-----| -> defined? ... -# 90| { ... } -#-----| -> call to each - # 90| x #-----| -> __synth__0__1 @@ -6915,7 +6915,7 @@ raise.rb: #-----| -> exit m # 167| m -#-----| -> exit raise.rb (normal) +#-----| -> m16 # 167| self #-----| -> m @@ -6928,3 +6928,74 @@ raise.rb: # 168| "" #-----| -> call to raise + +# 172| enter m16 +#-----| -> b1 + +# 172| exit m16 + +# 172| exit m16 (abnormal) +#-----| -> exit m16 + +# 172| exit m16 (normal) +#-----| -> exit m16 + +# 172| m16 +#-----| -> exit raise.rb (normal) + +# 172| b1 +#-----| -> b2 + +# 172| b2 +#-----| -> b1 + +# 174| b1 +#-----| true -> [true] ... || ... +#-----| false -> b2 + +# 174| ... || ... +#-----| true -> 1 +#-----| false -> 2 +#-----| raise -> ExceptionA + +# 174| [false] ... || ... +#-----| false -> 2 +#-----| raise -> ExceptionA + +# 174| [true] ... || ... +#-----| true -> 1 +#-----| raise -> ExceptionA + +# 174| b2 +#-----| -> true + +# 174| ... == ... +#-----| false -> [false] ... || ... +#-----| -> ... || ... +#-----| true -> [true] ... || ... +#-----| raise -> ExceptionA + +# 174| true +#-----| -> ... == ... + +# 175| return +#-----| return -> exit m16 (normal) + +# 175| 1 +#-----| -> return + +# 177| return +#-----| return -> exit m16 (normal) + +# 177| 2 +#-----| -> return + +# 179| ExceptionA +#-----| raise -> exit m16 (abnormal) +#-----| match -> 3 + +# 180| return +#-----| return -> exit m16 (normal) + +# 180| 3 +#-----| -> return diff --git a/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected b/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected index f7bc8a1b1e70..bc691d673b9a 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected +++ b/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected @@ -367,6 +367,10 @@ positionalArguments | raise.rb:160:5:162:7 | call to bar | raise.rb:160:9:162:7 | -> { ... } | | raise.rb:161:7:161:14 | call to raise | raise.rb:161:13:161:14 | "" | | raise.rb:168:5:168:12 | call to raise | raise.rb:168:11:168:12 | "" | +| raise.rb:174:8:174:23 | ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | +| raise.rb:174:8:174:23 | [false] ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | +| raise.rb:174:8:174:23 | [true] ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | +| raise.rb:174:14:174:23 | ... == ... | raise.rb:174:20:174:23 | true | keywordArguments | cfg.html.erb:6:9:6:58 | call to stylesheet_link_tag | media | cfg.html.erb:6:54:6:58 | "all" | | cfg.html.erb:12:11:12:33 | call to link_to | id | cfg.html.erb:12:31:12:33 | "a" | diff --git a/ruby/ql/test/library-tests/controlflow/graph/ifs.rb b/ruby/ql/test/library-tests/controlflow/graph/ifs.rb index 0e66c0da8cb3..f081095051f6 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/ifs.rb +++ b/ruby/ql/test/library-tests/controlflow/graph/ifs.rb @@ -55,4 +55,4 @@ def disjunct (b1, b2) if (b1 || b2) then puts "b1 or b2" end -end \ No newline at end of file +end diff --git a/ruby/ql/test/library-tests/controlflow/graph/raise.rb b/ruby/ql/test/library-tests/controlflow/graph/raise.rb index e5f0c0e50f5c..3caf234ab14c 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/raise.rb +++ b/ruby/ql/test/library-tests/controlflow/graph/raise.rb @@ -168,3 +168,15 @@ def self.m() raise "" end end + +def m16(b1, b2) + begin + if b1 || b2 == true + return 1 + else + return 2 + end + rescue ExceptionA + return 3 + end +end From ce3b359813d1cf44b5d6da16cff6704163c7118d Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 4 Apr 2024 13:03:08 +0200 Subject: [PATCH 2/2] Ruby: Fix CFG for nodes that may raise --- .../codeql/ruby/controlflow/internal/Completion.qll | 13 +++++++++---- .../library-tests/controlflow/graph/Cfg.expected | 6 ------ .../library-tests/controlflow/graph/Nodes.expected | 1 - 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll index 39fefcbeae1a..83ea11e9d230 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll @@ -90,9 +90,7 @@ private predicate mayRaise(Call c) { c = getARescuableBodyChild() } /** A completion of a statement or an expression. */ abstract class Completion extends TCompletion { - private predicate isValidForSpecific(AstNode n) { - exists(AstNode other | n = other.getDesugared() and this.isValidForSpecific(other)) - or + private predicate isValidForSpecific0(AstNode n) { this = n.(NonReturningCall).getACompletion() or completionIsValidForStmt(n, this) @@ -110,12 +108,19 @@ abstract class Completion extends TCompletion { or n = any(RescueModifierExpr parent).getBody() and this = [TSimpleCompletion().(TCompletion), TRaiseCompletion()] + } + + private predicate isValidForSpecific(AstNode n) { + this.isValidForSpecific0(n) + or + exists(AstNode other | n = other.getDesugared() and this.isValidForSpecific(other)) or mayRaise(n) and ( this = TRaiseCompletion() or - this = TSimpleCompletion() and not n instanceof NonReturningCall + not any(Completion c).isValidForSpecific0(n) and + this = TSimpleCompletion() ) } diff --git a/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected b/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected index 2571a9dfeeb4..dfb5c8dfee41 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -6953,11 +6953,6 @@ raise.rb: #-----| true -> [true] ... || ... #-----| false -> b2 -# 174| ... || ... -#-----| true -> 1 -#-----| false -> 2 -#-----| raise -> ExceptionA - # 174| [false] ... || ... #-----| false -> 2 #-----| raise -> ExceptionA @@ -6971,7 +6966,6 @@ raise.rb: # 174| ... == ... #-----| false -> [false] ... || ... -#-----| -> ... || ... #-----| true -> [true] ... || ... #-----| raise -> ExceptionA diff --git a/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected b/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected index bc691d673b9a..2bc683f894e4 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected +++ b/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected @@ -367,7 +367,6 @@ positionalArguments | raise.rb:160:5:162:7 | call to bar | raise.rb:160:9:162:7 | -> { ... } | | raise.rb:161:7:161:14 | call to raise | raise.rb:161:13:161:14 | "" | | raise.rb:168:5:168:12 | call to raise | raise.rb:168:11:168:12 | "" | -| raise.rb:174:8:174:23 | ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | | raise.rb:174:8:174:23 | [false] ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | | raise.rb:174:8:174:23 | [true] ... \|\| ... | raise.rb:174:14:174:23 | ... == ... | | raise.rb:174:14:174:23 | ... == ... | raise.rb:174:20:174:23 | true |