Merge pull request #11 from GitHubSecurityLab/fix_composite_actions

feat(composite-actions): Fix summary and source queries for composite actions analysis

Author: Alvaro Muñoz

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

@@ -139,31 +139,6 @@ newtype TContent =
     name = any(JobsCtxAccessExpr a).getFieldName()
   }
 
-/**
- * A reference contained in an object. Examples include instance fields, the
- * contents of a collection object, the contents of an array or pointer.
- */
-class Content extends TContent {
-  /** Gets the type of the contained data for the purpose of type pruning. */
-  DataFlowType getType() { any() }
-
-  /** Gets a textual representation of this element. */
-  abstract string toString();
-
-  /**
-   * Holds if this element is at the specified location.
-   * The location spans column `startcolumn` of line `startline` to
-   * column `endcolumn` of line `endline` in file `filepath`.
-   * For more information, see
-   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
-   */
-  predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    filepath = "" and startline = 0 and startcolumn = 0 and endline = 0 and endcolumn = 0
-  }
-}
-
 predicate forceHighPrecision(Content c) { c instanceof FieldContent }
 
 class ContentApprox = ContentSet;

ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll

@@ -130,6 +130,31 @@ class ContentSet instanceof Content {
   }
 }
 
+/**
+ * A reference contained in an object. Examples include instance fields, the
+ * contents of a collection object, the contents of an array or pointer.
+ */
+class Content extends TContent {
+  /** Gets the type of the contained data for the purpose of type pruning. */
+  DataFlowType getType() { any() }
+
+  /** Gets a textual representation of this element. */
+  abstract string toString();
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    filepath = "" and startline = 0 and startcolumn = 0 and endline = 0 and endcolumn = 0
+  }
+}
+
 /** A field of an object, for example an instance variable. */
 class FieldContent extends Content, TFieldContent {
   private string name;

ql/src/Security/CWE-020/CompositeActionSummaries.ql

@@ -17,12 +17,10 @@ import codeql.actions.dataflow.ExternalFlow
 
 private module MyConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
-    source instanceof DataFlow::ParameterNode and
     exists(CompositeActionStmt c | c.getInputsStmt().getInputExpr(_) = source.asExpr())
   }
 
   predicate isSink(DataFlow::Node sink) {
-    sink instanceof DataFlow::ReturnNode and
     exists(CompositeActionStmt c | c.getOutputsStmt().getOutputExpr(_) = sink.asExpr())
   }
 }

ql/src/Security/CWE-020/CompositeActionsSources.ql

@@ -23,9 +23,15 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate isSink(DataFlow::Node sink) {
-    sink instanceof DataFlow::ReturnNode and
     exists(CompositeActionStmt c | c.getOutputsStmt().getOutputExpr(_) = sink.asExpr())
   }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet set) {
+    allowImplicitRead(node, set)
+    or
+    isSink(node) and
+    set instanceof DataFlow::FieldContent
+  }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;