Port UI5PathGraph to use the newer data flow API

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll

@@ -168,7 +168,7 @@ class SapDefineModule extends AmdModuleDefinition::Range, MethodCallExpr, UserMo
     )
   }
 
-  string getDependency(int i) { result = this.(AmdModuleDefinition).getDependency(i).getValue() }
+  string getDependency(int i) { result = this.(AmdModuleDefinition).getDependencyExpr(i).getStringValue() }
 
   override string getADependency() { result = this.getDependency(_) }
 

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5FormulaInjectionQuery.qll

@@ -8,13 +8,13 @@ import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
 private class StoragePutCall extends CallNode {
   StoragePutCall() {
     /* 1. This is a call to `sap.ui.util.Storage.put` */
-    // 1-1. Required from `sap/ui/util/Storage`
+    /* 1-1. Required from `sap/ui/util/Storage` */
     exists(RequiredObject storageClass |
       this.getReceiver().getALocalSource() = storageClass.asSourceNode() and
       this.getCalleeName() = "put"
     )
     or
-    // 1-2. Direct call to `sap.ui.util.Storage.put`
+    /* 1-2. Direct call to `sap.ui.util.Storage.put` */
     this =
       globalVarRef("sap")
           .getAPropertyRead("ui")
@@ -109,12 +109,10 @@ private class FileSaveCall extends CallNode {
   }
 }
 
-class UI5FormulaInjectionConfiguration extends TaintTracking::Configuration {
-  UI5FormulaInjectionConfiguration() { this = "UI5 Formula Injection" }
+module UI5FormulaInjection implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     exists(StoragePutCall storagePutCall | node = storagePutCall.getArgument(1))
     or
     exists(FileSaveCall fileSaveCall |

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogInjectionQuery.qll

@@ -1,12 +1,13 @@
 import javascript
 import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
-private import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
-import semmle.javascript.security.dataflow.LogInjectionQuery as LogInjection
+import semmle.javascript.security.dataflow.LogInjectionQuery
 
-class UI5LogInjectionConfiguration extends LogInjection::LogInjectionConfiguration {
-  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+module UI5LogInjection implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
 
-  override predicate isSink(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) { LogInjectionConfig::isBarrier(node) }
+
+  predicate isSink(DataFlow::Node node) {
     node = ModelOutput::getASinkNode("ui5-log-injection").asSink()
   }
 }

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll

@@ -0,0 +1,43 @@
+import javascript
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+import advanced_security.javascript.frameworks.ui5.UI5LogInjectionQuery
+
+class ClientRequestInjectionVector extends DataFlow::Node {
+  ClientRequestInjectionVector() {
+    exists(ClientRequest req |
+      this = req.getUrl() or
+      this = req.getADataNode()
+    )
+  }
+}
+
+class UI5LogEntryFlowState extends string {
+  UI5LogEntryFlowState() { this = ["not-logged-not-accessed", "logged-and-accessed"] }
+}
+
+module UI5LogEntryToHttp implements DataFlow::StateConfigSig {
+  class FlowState = UI5LogEntryFlowState;
+
+  predicate isSource(DataFlow::Node node, FlowState state) {
+    node instanceof RemoteFlowSource and
+    state = "not-logged-not-accessed"
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node start, FlowState preState, DataFlow::Node end, FlowState postState
+  ) {
+    UI5LogInjection::isAdditionalFlowStep(start, end) and
+    preState = postState
+    or
+    exists(LogArgumentToListener logArgumentToListener |
+      logArgumentToListener.step(start, end) and
+      preState = "not-logged-not-accessed" and
+      postState = "logged-and-accessed"
+    )
+  }
+
+  predicate isSink(DataFlow::Node node, FlowState state) {
+    node instanceof ClientRequestInjectionVector and
+    state = "logged-and-accessed"
+  }
+}

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5PathInjectionQuery.qll

@@ -0,0 +1,9 @@
+import javascript
+
+module UI5PathInjection implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    node = ModelOutput::getASinkNode("ui5-path-injection").asSink()
+  }
+}

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5UnsafeLogAccessQuery.qll

@@ -0,0 +1,45 @@
+import javascript
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+import semmle.javascript.security.dataflow.LogInjectionQuery
+
+module UI5UnsafeLogAccess implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isBarrier(DataFlow::Node node) { LogInjectionConfig::isBarrier(node) }
+
+  predicate isSink(DataFlow::Node node) {
+    node = ModelOutput::getASinkNode("ui5-log-injection").asSink()
+  }
+}
+
+private newtype TLogEntriesNode =
+  TDataFlowNode(DataFlow::Node node) {
+    node = ModelOutput::getATypeNode("SapLogEntries").getInducingNode()
+  } or
+  TUI5ControlNode(UI5Control control) { control.getImportPath() = "sap/ui/vk/Notifications" }
+
+class LogEntriesNode extends TLogEntriesNode {
+  DataFlow::Node asDataFlowNode() { this = TDataFlowNode(result) }
+
+  UI5Control asUI5ControlNode() { this = TUI5ControlNode(result) }
+
+  File getFile() {
+    result = this.asDataFlowNode().getFile()
+    or
+    result = this.asUI5ControlNode().getView()
+  }
+
+  string toString() {
+    result = this.asDataFlowNode().toString()
+    or
+    result = this.asUI5ControlNode().toString()
+  }
+
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.asDataFlowNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    this.asUI5ControlNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5XssQuery.qll

@@ -1,39 +1,18 @@
 import javascript
 import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow as UI5DataFlow
 import advanced_security.javascript.frameworks.ui5.UI5View
-import semmle.javascript.security.dataflow.DomBasedXssQuery as DomBasedXss
+private import semmle.javascript.security.dataflow.DomBasedXssQuery as DomBasedXss
 
-class Configuration extends DomBasedXss::Configuration {
-  override predicate isSource(DataFlow::Node start) {
-    super.isSource(start)
+module UI5Xss implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node start) {
+    DomBasedXss::DomBasedXssConfig::isSource(start, _)
     or
     start instanceof RemoteFlowSource
   }
 
-  override predicate isAdditionalFlowStep(
-    DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel inLabel,
-    DataFlow::FlowLabel outLabel
-  ) {
-    /* Already an additional flow step defined in `DomBasedXssQuery::Configuration` */
-    super.isAdditionalFlowStep(start, end, inLabel, outLabel)
-    or
-    /* TODO: Legacy code */
-    /* Handler argument node to handler parameter */
-    exists(UI5Handler h |
-      start = h.getBindingPath().getNode() and
-      /*
-       * Ideally we would like to show an intermediate node where
-       * the handler is bound to a control, but there is no sourceNode there
-       * `end = h.getBindingPath() or start = h.getBindingPath()`
-       */
-
-      end = h.getParameter(0)
-    )
-  }
-
-  override predicate isBarrier(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     /* 1. Already a sanitizer defined in `DomBasedXssQuery::Configuration` */
-    super.isSanitizer(node)
+    DomBasedXss::DomBasedXssConfig::isBarrier(node)
     or
     /* 2. Value read from a non-string control property */
     exists(PropertyMetadata m | not m.isUnrestrictedStringType() | node = m)
@@ -53,10 +32,28 @@ class Configuration extends DomBasedXss::Configuration {
       ["encodeCSS", "encodeJS", "encodeURL", "encodeURLParameters", "encodeXML", "encodeHTML"]
   }
 
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     node instanceof UI5ExtHtmlISink or
     node instanceof UI5ModelHtmlISink
   }
+
+  predicate isAdditionalFlowStep(DataFlow::Node start, DataFlow::Node end) {
+    /* Already an additional flow step defined in `DomBasedXssQuery::Configuration` */
+    DomBasedXss::DomBasedXssConfig::isAdditionalFlowStep(start, _, end, _)
+    or
+    /* TODO: Legacy code */
+    /* Handler argument node to handler parameter */
+    exists(UI5Handler h |
+      start = h.getBindingPath().getNode() and
+      /*
+       * Ideally we would like to show an intermediate node where
+       * the handler is bound to a control, but there is no sourceNode there
+       * `end = h.getBindingPath() or start = h.getBindingPath()`
+       */
+
+      end = h.getParameter(0)
+    )
+  }
 }
 
 /**

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/DataFlow.qll

@@ -4,7 +4,6 @@ import advanced_security.javascript.frameworks.ui5.UI5
 import advanced_security.javascript.frameworks.ui5.UI5View
 import advanced_security.javascript.frameworks.ui5.RemoteFlowSources
 import advanced_security.javascript.frameworks.ui5.dataflow.FlowSteps
-private import StdLibDataFlow::DataFlow::PathGraph as DataFlowPathGraph
 private import PatchDataFlow
 
 /**
@@ -60,7 +59,7 @@ class LocalModelContentBoundBidirectionallyToHtmlISinkControl extends DomBasedXs
   UI5Control getControlDeclaration() { result = controlDeclaration }
 }
 
-module UI5PathGraph {
+module UI5PathGraph<PathNodeSig ConfigPathNode, PathGraphSig<ConfigPathNode> ConfigPathGraph> {
   private newtype TNode =
     TUI5BindingPathNode(UI5BindingPath path) or
     TDataFlowNode(DataFlow::Node node)
@@ -92,7 +91,7 @@ module UI5PathGraph {
           .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
     }
 
-    DataFlow::PathNode getPathNode() { result.getNode() = this.asDataFlowNode() }
+    ConfigPathNode getPathNode() { result.getNode() = this.asDataFlowNode() }
 
     UI5PathNode getAPrimarySource() {
       if this.asDataFlowNode() instanceof LocalModelContentBoundBidirectionallyToSourceControl
@@ -124,9 +123,9 @@ module UI5PathGraph {
   query predicate nodes(UI5PathNode ui5PathNode) {
     exists(ui5PathNode.asUI5BindingPathNode())
     or
-    exists(DataFlow::PathNode pathNode |
+    exists(ConfigPathNode pathNode |
       pathNode.getNode() = ui5PathNode.asDataFlowNode() and
-      DataFlowPathGraph::nodes(pathNode)
+      ConfigPathGraph::nodes(pathNode, _, _)
     )
   }
 
@@ -182,10 +181,10 @@ module UI5PathGraph {
 
   query predicate edges(UI5PathNode ui5PathNodePred, UI5PathNode ui5PathNodeSucc) {
     /* Include all existing dataflow edges */
-    exists(DataFlow::PathNode pathNodeFrom, DataFlow::PathNode pathNodeTo |
+    exists(ConfigPathNode pathNodeFrom, ConfigPathNode pathNodeTo |
       pathNodeFrom.getNode() = ui5PathNodePred.asDataFlowNode() and
       pathNodeTo.getNode() = ui5PathNodeSucc.asDataFlowNode() and
-      DataFlowPathGraph::edges(pathNodeFrom, pathNodeTo)
+      ConfigPathGraph::edges(pathNodeFrom, pathNodeTo, _, _)
     ) and
     /* ========= TODO: Legacy code ========= */
     /* Exclude duplicate edge from model to handler parameter */

javascript/frameworks/ui5/src/UI5FormulaInjection/UI5FormulaInjection.ql

@@ -13,14 +13,21 @@
 
 import javascript
 import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
-import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
 import advanced_security.javascript.frameworks.ui5.UI5FormulaInjectionQuery
 
+module UI5FormulaInjectionFlow = TaintTracking::Global<UI5FormulaInjection>;
+
+module UI5FormulaInjectionUI5PathGraph =
+  UI5PathGraph<UI5FormulaInjectionFlow::PathNode, UI5FormulaInjectionFlow::PathGraph>;
+
+import UI5FormulaInjectionUI5PathGraph
+
 from
-  UI5FormulaInjectionConfiguration config, UI5PathNode source, UI5PathNode sink,
-  UI5PathNode primarySource
+  UI5FormulaInjectionUI5PathGraph::UI5PathNode source,
+  UI5FormulaInjectionUI5PathGraph::UI5PathNode sink,
+  UI5FormulaInjectionUI5PathGraph::UI5PathNode primarySource
 where
-  config.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
+  UI5FormulaInjectionFlow::flowPath(source.getPathNode(), sink.getPathNode()) and
   primarySource = source.getAPrimarySource()
 select sink, primarySource, sink, "The content of a saved file depends on a $@.", primarySource,
   "user-provided value"

javascript/frameworks/ui5/src/UI5LogInjection/UI5LogInjection.ql

@@ -13,11 +13,19 @@
 
 import javascript
 import advanced_security.javascript.frameworks.ui5.UI5LogInjectionQuery
-import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+
+module UI5LogInjectionFlow = TaintTracking::Global<UI5LogInjection>;
+
+module UI5LogInjectionUI5PathGraph =
+  UI5PathGraph<UI5LogInjectionFlow::PathNode, UI5LogInjectionFlow::PathGraph>;
+
+import UI5LogInjectionUI5PathGraph
 
 from
-  UI5LogInjectionConfiguration cfg, UI5PathNode source, UI5PathNode sink, UI5PathNode primarySource
+  UI5LogInjectionUI5PathGraph::UI5PathNode source, UI5LogInjectionUI5PathGraph::UI5PathNode sink,
+  UI5LogInjectionUI5PathGraph::UI5PathNode primarySource
 where
-  cfg.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
+  UI5LogInjectionFlow::flowPath(source.getPathNode(), sink.getPathNode()) and
   primarySource = source.getAPrimarySource()
 select sink, primarySource, sink, "Log entry depends on a $@.", primarySource, "user-provided value"