Merge pull request #12821 from maikypedia/maikypedia/ruby-ssti

alexrford · web-flow · commit 388b2abf68e2 · 2023-05-02T16:56:27.000+01:00
Ruby: Add Rails `render inline:` as Template Injection Sink
diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll b/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll
@@ -400,3 +400,19 @@ private class AccessLocalsKeySummary extends SummarizedCallable {
     preservesValue = true
   }
 }
+
+/** A call to `render inline: foo`, considered as a ERB template rendering. */
+private class RailsTemplateRendering extends TemplateRendering::Range, DataFlow::CallNode {
+  private DataFlow::Node template;
+
+  RailsTemplateRendering() {
+    (
+      this.asExpr().getExpr() instanceof Rails::RenderCall
+      or
+      this.asExpr().getExpr() instanceof Rails::RenderToCall
+    ) and
+    template = this.getKeywordArgument("inline")
+  }
+
+  override DataFlow::Node getTemplate() { result = template }
+}
diff --git a/ruby/ql/src/experimental/template-injection/examples/SSTIBad.rb b/ruby/ql/src/experimental/template-injection/examples/SSTIBad.rb
@@ -9,6 +9,7 @@ def some_request_handler
       <h2>Hello %s </h2></body></html>
       " % name
     template = ERB.new(html_text).result(binding) 
+    render inline: html_text
   end
 end
 
diff --git a/ruby/ql/src/experimental/template-injection/examples/SSTIGood.rb b/ruby/ql/src/experimental/template-injection/examples/SSTIGood.rb
@@ -9,6 +9,7 @@ def some_request_handler
       <h2>Hello <%= name %> </h2></body></html>
       "
     template = ERB.new(html_text).result(binding) 
+    render inline: html_text
   end
 end
 
diff --git a/ruby/ql/test/query-tests/experimental/TemplateInjection/ErbInjection.rb b/ruby/ql/test/query-tests/experimental/TemplateInjection/ErbInjection.rb
@@ -14,6 +14,10 @@ def some_request_handler
     # where name is unsanitized
     template = ERB.new(bad_text).result(binding) 
 
+    # BAD: user input is evaluated
+    # where name is unsanitized
+    render inline: bad_text
+
     # Template with the source 
     good_text = "
       <!DOCTYPE html><html><body>
@@ -22,6 +26,9 @@ def some_request_handler
 
     # GOOD: user input is not evaluated
     template2 = ERB.new(good_text).result(binding) 
+
+    # GOOD: user input is not evaluated
+    render inline: good_text
   end
 end
 
diff --git a/ruby/ql/test/query-tests/experimental/TemplateInjection/TemplateInjection.expected b/ruby/ql/test/query-tests/experimental/TemplateInjection/TemplateInjection.expected
@@ -4,6 +4,7 @@ edges
 | ErbInjection.rb:5:12:5:17 | call to params | ErbInjection.rb:5:12:5:24 | ...[...] |
 | ErbInjection.rb:5:12:5:24 | ...[...] | ErbInjection.rb:5:5:5:8 | name |
 | ErbInjection.rb:8:5:8:12 | bad_text | ErbInjection.rb:15:24:15:31 | bad_text |
+| ErbInjection.rb:8:5:8:12 | bad_text | ErbInjection.rb:19:20:19:27 | bad_text |
 | ErbInjection.rb:8:16:11:14 | ... % ... | ErbInjection.rb:8:5:8:12 | bad_text |
 | ErbInjection.rb:11:11:11:14 | name | ErbInjection.rb:8:16:11:14 | ... % ... |
 | SlimInjection.rb:5:5:5:8 | name | SlimInjection.rb:8:5:8:12 | bad_text |
@@ -23,6 +24,7 @@ nodes
 | ErbInjection.rb:8:16:11:14 | ... % ... | semmle.label | ... % ... |
 | ErbInjection.rb:11:11:11:14 | name | semmle.label | name |
 | ErbInjection.rb:15:24:15:31 | bad_text | semmle.label | bad_text |
+| ErbInjection.rb:19:20:19:27 | bad_text | semmle.label | bad_text |
 | SlimInjection.rb:5:5:5:8 | name | semmle.label | name |
 | SlimInjection.rb:5:12:5:17 | call to params | semmle.label | call to params |
 | SlimInjection.rb:5:12:5:24 | ...[...] | semmle.label | ...[...] |
@@ -35,5 +37,6 @@ nodes
 subpaths
 #select
 | ErbInjection.rb:15:24:15:31 | bad_text | ErbInjection.rb:5:12:5:17 | call to params | ErbInjection.rb:15:24:15:31 | bad_text | This template depends on a $@. | ErbInjection.rb:5:12:5:17 | call to params | user-provided value |
+| ErbInjection.rb:19:20:19:27 | bad_text | ErbInjection.rb:5:12:5:17 | call to params | ErbInjection.rb:19:20:19:27 | bad_text | This template depends on a $@. | ErbInjection.rb:5:12:5:17 | call to params | user-provided value |
 | SlimInjection.rb:14:25:14:32 | bad_text | SlimInjection.rb:5:12:5:17 | call to params | SlimInjection.rb:14:25:14:32 | bad_text | This template depends on a $@. | SlimInjection.rb:5:12:5:17 | call to params | user-provided value |
 | SlimInjection.rb:23:25:23:33 | bad2_text | SlimInjection.rb:5:12:5:17 | call to params | SlimInjection.rb:23:25:23:33 | bad2_text | This template depends on a $@. | SlimInjection.rb:5:12:5:17 | call to params | user-provided value |

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ def some_request_handler`
`9`	`9`	`<h2>Hello <%= name %> </h2></body></html>`
`10`	`10`	`"`
`11`	`11`	`template = ERB.new(html_text).result(binding)`
	`12`	`+ render inline: html_text`
`12`	`13`	`end`
`13`	`14`	`end`
`14`	`15`