OAuth2 exclusion: hide cases that clearly target an out-of-band process or private HTTP server

Author: smowton

ql/src/experimental/CWE-352/ConstantOauth2State.ql

@@ -40,6 +40,58 @@ class ConstantStateFlowConf extends DataFlow::Configuration {
   override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
 }
 
+/**
+ * A flow of a URL indicating the OAuth redirect doesn't point to a publically
+ * accessible address, to the receiver of an AuthCodeURL call.
+ *
+ * Note we accept localhost and 127.0.0.1 on the assumption this is probably a transient
+ * listener; if it actually is a persistent server then that really is vulnerable to CSRF.
+ */
+class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration {
+  PrivateUrlFlowsToAuthCodeUrlCall() { this = "PrivateUrlFlowsToConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    // The following are all common ways to indicate out-of-band OAuth2 flow, in which case
+    // the authenticating party does not redirect but presents a code for the user to copy
+    // instead.
+    source.asExpr().(StringLit).getValue() in ["urn:ietf:wg:oauth:2.0:oob",
+          "urn:ietf:wg:oauth:2.0:oob:auto", "oob", "code"] or
+    // Alternatively some non-web tools will create a temporary local webserver to handle the
+    // OAuth2 redirect:
+    source.asExpr().(StringLit).getValue().matches("%://localhost%") or
+    source.asExpr().(StringLit).getValue().matches("%://127.0.0.1%")
+  }
+
+  /**
+   * Propagates a URL written to a RedirectURL field to the whole Config object.
+   */
+  override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(Write w, Field f | f.hasQualifiedName("golang.org/x/oauth2", "Config", "RedirectURL") |
+      w.writesField(succ.(DataFlow::PostUpdateNode).getPreUpdateNode(), f, pred)
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) {
+    exists(AuthCodeURL m | call = m.getACall() | sink = call.getReceiver())
+  }
+
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+}
+
+/**
+ * Holds if a URL indicating the OAuth redirect doesn't point to a publically
+ * accessible address, to the receiver of an AuthCodeURL call.
+ *
+ * Note we accept localhost and 127.0.0.1 on the assumption this is probably a transient
+ * listener; if it actually is a persistent server then that really is vulnerable to CSRF.
+ */
+predicate privateUrlFlowsToAuthCodeUrlCall(DataFlow::CallNode call) {
+  exists(PrivateUrlFlowsToAuthCodeUrlCall flowConfig, DataFlow::Node receiver |
+    flowConfig.hasFlowTo(receiver) and
+    flowConfig.isSink(receiver, call)
+  )
+}
+
 /** A flow to a printer function of the fmt package. */
 class FlowToPrint extends DataFlow::Configuration {
   FlowToPrint() { this = "FlowToPrint" }
@@ -108,6 +160,7 @@ where
   cfg.hasFlowPath(source, sink) and
   cfg.isSink(sink.getNode(), sinkCall) and
   // Exclude cases that seem to be oauth flows done from within a terminal:
-  not seemsLikeDoneWithinATerminal(sinkCall)
+  not seemsLikeDoneWithinATerminal(sinkCall) and
+  not privateUrlFlowsToAuthCodeUrlCall(sinkCall)
 select sink.getNode(), source, sink, "Using a constant $@ to create oauth2 URLs.", source.getNode(),
   "state string"