Ruby: tests, patterns, fix erb flow

Author: hmac

ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll

@@ -8,13 +8,13 @@ private import codeql.ruby.dataflow.FlowSummary
 
 /** Provides modeling for the Sinatra library. */
 module Sinatra {
-  private class App extends DataFlow::ClassNode {
+  class App extends DataFlow::ClassNode {
     App() { this = DataFlow::getConstant("Sinatra").getConstant("Base").getADescendentModule() }
 
-    Route getRoute() { result.getApp() = this }
+    Route getARoute() { result.getApp() = this }
   }
 
-  private class Route extends DataFlow::CallNode {
+  class Route extends DataFlow::CallNode {
     private App app;
 
     Route() {
@@ -44,22 +44,26 @@ module Sinatra {
     }
   }
 
-  private class ErbCall extends DataFlow::CallNode {
+  class ErbCall extends DataFlow::CallNode {
     private Route route;
 
     ErbCall() {
       this.asExpr().getExpr().getEnclosingCallable() = route.getBody().asCallableAstNode() and
       this.getMethodName() = "erb"
     }
 
-    ErbFile getTemplateFile() {
-      result.getTemplateName() =
-        this.getArgument(0).asExpr().getConstantValue().getStringlikeValue() and
-      result.getRelativePath().matches("%views/%")
-    }
+    /**
+     * Gets the template file corresponding to this call.
+     */
+    ErbFile getTemplateFile() { result = getTemplateFile(this.asExpr().getExpr()) }
   }
 
-  ErbFile getTemplateFile(MethodCall erbCall) {
+  /**
+   * Gets the template file referred to by `erbCall`.
+   * This works on the AST level to avoid non-monotonic reecursion in `ErbLocalsHashSyntheticGlobal`.
+   */
+  private ErbFile getTemplateFile(MethodCall erbCall) {
+    erbCall.getMethodName() = "erb" and
     result.getTemplateName() = erbCall.getArgument(0).getConstantValue().getStringlikeValue() and
     result.getRelativePath().matches("%views/%")
   }
@@ -73,7 +77,7 @@ module Sinatra {
         loc.getEndLine() + ":" + loc.getEndColumn()
   }
 
-  private class ErbLocalsHashSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
+  class ErbLocalsHashSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
     private string id;
     private MethodCall erbCall;
     private ErbFile erbFile;
@@ -98,7 +102,9 @@ module Sinatra {
     override MethodCall getACall() { result = any(ErbCall c).asExpr().getExpr() }
 
     override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
-      input = "Argument[2]" and output = "SyntheticGlobal[" + global + "]" and preservesValue = true
+      input = "Argument[locals:]" and
+      output = "SyntheticGlobal[" + global + "]" and
+      preservesValue = true
     }
   }
 
@@ -124,4 +130,43 @@ module Sinatra {
       preservesValue = true
     }
   }
+
+  /**
+   * Filters are run before or after the route handler. They can modify the
+   * request and response, and share instance variables with the route handler.
+   */
+  class Filter extends DataFlow::CallNode {
+    private App app;
+
+    Filter() { this = app.getAModuleLevelCall(["before", "after"]) }
+
+    /**
+     * Gets the pattern which constrains this route, if any. In the example below, the pattern is `/protected/*`.
+     * Patterns are typically given as strings, and are interpreted by the `mustermann` gem (they are not regular expressions).
+     * ```rb
+     * before '/protected/*' do
+     *   authenticate!
+     * end
+     * ```
+     */
+    DataFlow::ExprNode getPattern() { result = this.getArgument(0) }
+
+    /**
+     * Holds if this filter has a pattern.
+     */
+    predicate hasPattern() { exists(this.getPattern()) }
+
+    /**
+     * Gets the body of this filter.
+     */
+    DataFlow::BlockNode getBody() { result = this.getBlock() }
+  }
+
+  class BeforeFilter extends Filter {
+    BeforeFilter() { this.getMethodName() = "before" }
+  }
+
+  class AfterFilter extends Filter {
+    AfterFilter() { this.getMethodName() = "after" }
+  }
 }

ruby/ql/test/library-tests/frameworks/sinatra/Flow.expected

@@ -0,0 +1,18 @@
+failures
+| views/index.erb:2:10:2:12 | call to foo | Unexpected result: hasTaintFlow= |
+edges
+| app.rb:75:5:75:8 | [post] self [@foo] :  | app.rb:76:32:76:35 | self [@foo] :  |
+| app.rb:75:12:75:17 | call to params :  | app.rb:75:12:75:24 | ...[...] :  |
+| app.rb:75:12:75:24 | ...[...] :  | app.rb:75:5:75:8 | [post] self [@foo] :  |
+| app.rb:76:32:76:35 | @foo :  | views/index.erb:2:10:2:12 | call to foo |
+| app.rb:76:32:76:35 | self [@foo] :  | app.rb:76:32:76:35 | @foo :  |
+nodes
+| app.rb:75:5:75:8 | [post] self [@foo] :  | semmle.label | [post] self [@foo] :  |
+| app.rb:75:12:75:17 | call to params :  | semmle.label | call to params :  |
+| app.rb:75:12:75:24 | ...[...] :  | semmle.label | ...[...] :  |
+| app.rb:76:32:76:35 | @foo :  | semmle.label | @foo :  |
+| app.rb:76:32:76:35 | self [@foo] :  | semmle.label | self [@foo] :  |
+| views/index.erb:2:10:2:12 | call to foo | semmle.label | call to foo |
+subpaths
+#select
+| views/index.erb:2:10:2:12 | call to foo | app.rb:75:12:75:17 | call to params :  | views/index.erb:2:10:2:12 | call to foo | $@ | app.rb:75:12:75:17 | call to params :  | call to params :  |

ruby/ql/test/library-tests/frameworks/sinatra/Flow.ql

@@ -0,0 +1,17 @@
+/**
+ * @kind path-problem
+ */
+
+import ruby
+import TestUtilities.InlineFlowTest
+import PathGraph
+import codeql.ruby.frameworks.Sinatra
+import codeql.ruby.Concepts
+
+class ParamsTaintFlowConf extends DefaultTaintFlowConf {
+  override predicate isSource(DataFlow::Node n) { n instanceof Http::Server::RequestInputAccess }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ParamsTaintFlowConf conf
+where conf.hasFlowPath(source, sink)
+select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.expected

@@ -0,0 +1,42 @@
+routes
+| app.rb:1:1:105:3 | MyApp | app.rb:2:3:4:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:6:3:8:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:10:3:13:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:15:3:18:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:20:3:22:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:24:3:26:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:28:3:31:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:33:3:35:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:37:3:42:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:44:3:46:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:48:3:50:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:52:3:54:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:56:3:58:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:60:3:62:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:66:3:68:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:70:3:72:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:74:3:77:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:79:3:82:5 | call to get |
+| app.rb:1:1:105:3 | MyApp | app.rb:89:3:92:5 | call to get |
+params
+| app.rb:3:14:3:19 | call to params |
+| app.rb:12:5:12:10 | call to params |
+| app.rb:17:5:17:10 | call to params |
+| app.rb:25:15:25:20 | call to params |
+| app.rb:39:13:39:18 | call to params |
+| app.rb:40:14:40:19 | call to params |
+| app.rb:45:38:45:43 | call to params |
+| app.rb:75:12:75:17 | call to params |
+| app.rb:91:5:91:10 | call to params |
+erbCalls
+| app.rb:76:5:76:36 | call to erb | views/index.erb:0:0:0:0 | views/index.erb |
+erbSyntheticGlobals
+| SinatraErbLocalsHash(library-tests/frameworks/sinatra/views/index.erb,library-tests/frameworks/sinatra/app.rb@76:5:76:36) | views/index.erb:0:0:0:0 | views/index.erb |
+filters
+| app.rb:84:3:87:5 | call to before | before |
+| app.rb:94:3:96:5 | call to after | after |
+| app.rb:98:3:100:5 | call to before | before |
+| app.rb:102:3:104:5 | call to after | after |
+filterPatterns
+| app.rb:98:3:100:5 | call to before | app.rb:98:10:98:23 | "/protected/*" |
+| app.rb:102:3:104:5 | call to after | app.rb:102:9:102:23 | "/create/:slug" |

ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.ql

@@ -0,0 +1,26 @@
+import ruby
+import codeql.ruby.frameworks.Sinatra
+import codeql.ruby.Concepts
+import codeql.ruby.AST
+
+query predicate routes(Sinatra::App app, Sinatra::Route route) { route = app.getARoute() }
+
+query predicate params(Http::Server::RequestInputAccess params) { any() }
+
+query predicate erbCalls(Sinatra::ErbCall c, ErbFile templateFile) {
+  templateFile = c.getTemplateFile()
+}
+
+query predicate erbSyntheticGlobals(Sinatra::ErbLocalsHashSyntheticGlobal g, ErbFile file) {
+  file = g.getErbFile()
+}
+
+query predicate filters(Sinatra::Filter filter, string kind) {
+  filter instanceof Sinatra::BeforeFilter and kind = "before"
+  or
+  filter instanceof Sinatra::AfterFilter and kind = "after"
+}
+
+query predicate filterPatterns(Sinatra::Filter filter, DataFlow::ExprNode pattern) {
+  pattern = filter.getPattern()
+}

ruby/ql/test/library-tests/frameworks/sinatra/app.rb

@@ -0,0 +1,106 @@
+class MyApp < Sinatra::Base
+  get '/hello/:name' do
+    "Hello #{params['name']}!"
+  end
+  
+  get '/goodbye/:name' do |n|
+    "Goodbyte #{n}!"
+  end
+  
+  get '/say/*/to/*' do
+    # matches /say/hello/to/world
+    params['splat'] # => ["hello", "world"]
+  end
+  
+  get '/download/*.*' do
+    # matches /download/path/to/file.xml
+    params['splat'] # => ["path/to/file", "xml"]
+  end
+  
+  get '/download/*.*' do |path, ext|
+    [path, ext] # => ["path/to/file", "xml"]
+  end
+  
+  get /\/hello\/([\w]+)/ do
+    "Hello, #{params['captures'].first}!"
+  end
+  
+  get %r{/hello/([\w]+)} do |c|
+    # Matches "GET /meta/hello/world", "GET /hello/world/1234" etc.
+    "Hello, #{c}!"
+  end
+  
+  get '/posts/:format?' do
+    # matches "GET /posts/" and any extension "GET /posts/json", "GET /posts/xml" etc
+  end
+  
+  get '/posts' do
+    # matches "GET /posts?title=foo&author=bar"
+    title = params['title']
+    author = params['author']
+    # uses title and author variables; query is optional to the /posts route
+  end
+  
+  get '/foo', :agent => /Songbird (\d\.\d)[\d\/]*?/ do
+    "You're using Songbird version #{params['agent'][0]}"
+  end
+  
+  get '/foo' do
+    # Matches non-songbird browsers
+  end
+  
+  get '/', :host_name => /^admin\./ do
+    "Admin Area, Access denied!"
+  end
+  
+  get '/', :provides => 'html' do
+    haml :index
+  end
+  
+  get '/', :provides => ['rss', 'atom', 'xml'] do
+    builder :feed
+  end
+  
+  set(:probability) { |value| condition { rand <= value } }
+
+  get '/win_a_car', :probability => 0.1 do
+    "You won!"
+  end
+
+  get '/win_a_car' do
+    "Sorry, you lost."
+  end
+  
+  get '/' do
+    @foo = params["foo"]
+    erb :index, locals: {foo:  @foo}
+  end
+  
+  get '/' do
+    code = "<%= Time.now %>"
+    erb code
+  end
+  
+  before do
+    @note = 'Hi!'
+    request.path_info = '/foo/bar/baz'
+  end
+  
+  get '/foo/*' do
+    @note #=> 'Hi!'
+    params['splat'] #=> 'bar/baz'
+  end
+  
+  after do
+    puts response.status
+  end
+  
+  before '/protected/*' do
+    authenticate!
+  end
+  
+  after '/create/:slug' do |slug|
+    session[:last_slug] = slug
+  end
+end
+

ruby/ql/test/library-tests/frameworks/sinatra/views/index.erb

@@ -0,0 +1,2 @@
+<%= @foo %>
+<%= sink foo %>