Merge pull request #7335 from hvitved/ruby/dataflow/hide-desugared-nodes

Ruby: Hide desugared nodes in data-flow paths

Author: hvitved

ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll

@@ -126,6 +126,23 @@ private class Desugared extends AstNode {
  */
 int desugarLevel(AstNode n) { result = count(Desugared desugared | n = desugared.getADescendant()) }
 
+/**
+ * Holds if `n` appears in a context that is desugared. That is, a
+ * transitive, reflexive parent of `n` is a desugared node.
+ */
+predicate isInDesugeredContext(AstNode n) { n = any(AstNode sugar).getDesugared().getAChild*() }
+
+/**
+ * Holds if `n` is a node that only exists as a result of desugaring some
+ * other node.
+ */
+predicate isDesugarNode(AstNode n) {
+  n = any(AstNode sugar).getDesugared()
+  or
+  isInDesugeredContext(n) and
+  forall(AstNode parent | parent = n.getParent() | parent.isSynthesized())
+}
+
 /**
  * Use this predicate in `Synthesis::child` to generate an assignment of `value` to
  * synthesized variable `v`, where the assignment is a child of `assignParent` at

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.ast.internal.Synthesis
 private import codeql.ruby.CFG
 private import codeql.ruby.dataflow.SSA
 private import DataFlowPublic
@@ -279,6 +280,8 @@ predicate nodeIsHidden(Node n) {
     def instanceof Ssa::PhiNode
   )
   or
+  isDesugarNode(n.(ExprNode).getExprNode().getExpr())
+  or
   n instanceof SummaryNode
   or
   n instanceof SummaryParameterNode

ruby/ql/lib/codeql/ruby/printAst.qll

@@ -8,13 +8,7 @@
 
 private import AST
 private import codeql.ruby.security.performance.RegExpTreeView as RETV
-
-/** Holds if `n` appears in the desugaring of some other node. */
-predicate isDesugared(AstNode n) {
-  n = any(AstNode sugar).getDesugared()
-  or
-  isDesugared(n.getParent())
-}
+private import codeql.ruby.ast.internal.Synthesis
 
 /**
  * The query can extend this class to control which nodes are printed.
@@ -25,19 +19,7 @@ class PrintAstConfiguration extends string {
   /**
    * Holds if the given node should be printed.
    */
-  predicate shouldPrintNode(AstNode n) {
-    not isDesugared(n)
-    or
-    not n.isSynthesized()
-    or
-    n.isSynthesized() and
-    not n = any(AstNode sugar).getDesugared() and
-    exists(AstNode parent |
-      parent = n.getParent() and
-      not parent.isSynthesized() and
-      not n = parent.getDesugared()
-    )
-  }
+  predicate shouldPrintNode(AstNode n) { not isDesugarNode(n) }
 
   predicate shouldPrintAstEdge(AstNode parent, string edgeName, AstNode child) {
     child = parent.getAChild(edgeName) and

ruby/ql/test/library-tests/ast/AstDesugar.ql

@@ -8,7 +8,7 @@ import codeql.ruby.ast.internal.Synthesis
 
 class DesugarPrintAstConfiguration extends PrintAstConfiguration {
   override predicate shouldPrintNode(AstNode n) {
-    isDesugared(n)
+    isInDesugeredContext(n)
     or
     exists(n.getDesugared())
   }

ruby/ql/test/query-tests/security/cwe-079/ReflectedXSS.expected

@@ -1,8 +1,7 @@
 edges
 | app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params :  | app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  |
 | app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  | app/views/foo/bars/show.html.erb:47:5:47:13 | call to user_name |
-| app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  | app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo |
-| app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  |
+| app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo |
 | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params :  | app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  |
 | app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website |
 | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/controllers/foo/bars_controller.rb:19:22:19:23 | dt :  |
@@ -21,7 +20,6 @@ edges
 nodes
 | app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params :  | semmle.label | call to params :  |
 | app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  | semmle.label | ...[...] :  |
-| app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  | semmle.label | ... = ... :  |
 | app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | semmle.label | call to params :  |
 | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params :  | semmle.label | call to params :  |
 | app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  | semmle.label | ...[...] :  |