CWE-352: Use of constant state in Oauth2 flow

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

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+            OAuth 2.0 clients must implement CSRF protection for the redirection URI, which is typically accomplished by including a "state" value that binds the request to
+            the user's authenticated state. The Go OAuth 2.0 library allows to specify a "state" value which is then included in the auth code URL, and then provided back by the remote authentication server in the redirect callback, from where it must be validated; failure to do so makes the client susceptible to an CSRF attack.
+        </p>
+    </overview>
+    <recommendation>
+        <p>
+            Always include a unique, non-guessable <code>state</code> value (provided to the call to <code>AuthCodeURL</code> function) that is also bound to the user's authenticated state with each authentication request, and then validated in the redirect callback.
+        </p>
+    </recommendation>
+    <example>
+        <p>
+            The first example shows you the use of a constant state (bad).
+        </p>
+        <sample src="ConstantOauth2StateBad.go" />
+        <p>
+            The second example shows a better implementation idea.
+        </p>
+        <sample src="ConstantOauth2StateBetter.go" />
+    </example>
+</qhelp>

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

@@ -0,0 +1,101 @@
+/**
+ * @name Use of constant `state` value in OAuth 2.0 URL.
+ * @description Using a constant value for the `state` in the OAuth 2.0 URL makes the application
+ *              susceptible to CSRF attacks.
+ * @kind path-problem
+ * @problem.severity error
+ * @id go/constant-oauth2-state
+ * @tags security
+ *       external/cwe/cwe-352
+ */
+
+import go
+import DataFlow::PathGraph
+
+/**
+ * A method that creates a new URL that will send the user
+ * to the OAuth 2.0 authorization dialog of the provider.
+ */
+class AuthCodeURL extends Method {
+  AuthCodeURL() { this.hasQualifiedName("golang.org/x/oauth2", "Config", "AuthCodeURL") }
+}
+
+/**
+ * A flow of a constant string value to a call to AuthCodeURL as the
+ * `state` parameter.
+ */
+class ConstantStateFlowConf extends DataFlow::Configuration {
+  ConstantStateFlowConf() { this = "ConstantStateFlowConf" }
+
+  predicate isSource(DataFlow::Node source, Literal state) {
+    state.isConst() and source.asExpr() = state
+  }
+
+  predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) {
+    exists(AuthCodeURL m | call = m.getACall() | sink = call.getArgument(0))
+  }
+
+  override predicate isSource(DataFlow::Node source) { isSource(source, _) }
+
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+}
+
+/** A flow to a printer function of the fmt package. */
+class FlowToPrint extends DataFlow::Configuration {
+  FlowToPrint() { this = "FlowToPrint" }
+
+  predicate isSource(DataFlow::Node source, DataFlow::CallNode call) {
+    exists(AuthCodeURL m | call = m.getACall() | source = call.getResult())
+  }
+
+  predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) {
+    exists(Fmt::Printer printer | call = printer.getACall() | sink = call.getArgument(_))
+  }
+
+  override predicate isSource(DataFlow::Node source) { isSource(source, _) }
+
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+}
+
+/** Holds if the provided CallNode's result flows to a Printer call as argument. */
+predicate resultFlowsToPrinter(DataFlow::CallNode authCodeURLCall) {
+  exists(FlowToPrint cfg, DataFlow::PathNode source, DataFlow::PathNode sink |
+    cfg.hasFlowPath(source, sink) and
+    cfg.isSource(source.getNode(), authCodeURLCall)
+  )
+}
+
+/**
+ * Holds if the provided CallNode is within the same root as a call
+ * to a scanner that reads from os.Stdin.
+ */
+predicate rootContainsCallToStdinScanner(DataFlow::CallNode authCodeURLCall) {
+  exists(Fmt::ScannerCall scannerCall | scannerCall.getRoot() = authCodeURLCall.getRoot())
+  or
+  exists(Fmt::FScannerCall fScannerCall |
+    fScannerCall.getReader() = any(ValueEntity v | v.hasQualifiedName("os", "Stdin")).getARead() and
+    fScannerCall.getRoot() = authCodeURLCall.getRoot()
+  )
+}
+
+/**
+ * Holds if the authCodeURLCall seems to be done within a terminal
+ * because there are calls to a Printer (fmt.Println and similar),
+ * and a call to a Scanner (fmt.Scan and similar),
+ * all of which are typically done within a terminal session.
+ */
+predicate seemsLikeDoneWithinATerminal(DataFlow::CallNode authCodeURLCall) {
+  resultFlowsToPrinter(authCodeURLCall) and
+  rootContainsCallToStdinScanner(authCodeURLCall)
+}
+
+from
+  ConstantStateFlowConf cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
+  DataFlow::CallNode sinkCall
+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)
+select sink.getNode(), source, sink, "Using a constant $@ to create oauth2 URLs.", source.getNode(),
+  "state string"

ql/src/experimental/CWE-352/ConstantOauth2StateBad.go

@@ -0,0 +1,24 @@
+package main
+
+import (
+	"golang.org/x/oauth2"
+)
+
+func main() {}
+
+var stateStringVar = "state"
+
+func badWithStringLiteralState() {
+	conf := &oauth2.Config{
+		ClientID:     "YOUR_CLIENT_ID",
+		ClientSecret: "YOUR_CLIENT_SECRET",
+		Scopes:       []string{"SCOPE1", "SCOPE2"},
+		Endpoint: oauth2.Endpoint{
+			AuthURL:  "https://provider.com/o/oauth2/auth",
+			TokenURL: "https://provider.com/o/oauth2/token",
+		},
+	}
+
+	url := conf.AuthCodeURL(stateStringVar)
+	// ...
+}

ql/src/experimental/CWE-352/ConstantOauth2StateBetter.go

@@ -0,0 +1,35 @@
+package main
+
+import (
+	"crypto/rand"
+	"encoding/base64"
+	"net/http"
+
+	"golang.org/x/oauth2"
+)
+
+func betterWithVariableStateReturned(w http.ResponseWriter) {
+	conf := &oauth2.Config{
+		ClientID:     "YOUR_CLIENT_ID",
+		ClientSecret: "YOUR_CLIENT_SECRET",
+		Scopes:       []string{"SCOPE1", "SCOPE2"},
+		Endpoint: oauth2.Endpoint{
+			AuthURL:  "https://provider.com/o/oauth2/auth",
+			TokenURL: "https://provider.com/o/oauth2/token",
+		},
+	}
+
+	state := generateStateOauthCookie(w)
+	url := conf.AuthCodeURL(state)
+	_ = url
+	// ...
+}
+func generateStateOauthCookie(w http.ResponseWriter) string {
+	b := make([]byte, 128)
+	rand.Read(b)
+	// TODO: save the state string to cookies or HTML storage,
+	// and bind it to the authenticated status of the user.
+	state := base64.URLEncoding.EncodeToString(b)
+
+	return state
+}

ql/src/semmle/go/frameworks/Stdlib.qll

@@ -94,7 +94,7 @@ module Fmt {
   }
 
   /** The `Print` function or one of its variants. */
-  private class Printer extends Function {
+  class Printer extends Function {
     Printer() { this.hasQualifiedName("fmt", ["Print", "Printf", "Println"]) }
   }
 
@@ -131,6 +131,35 @@ module Fmt {
       exists(int i | if getName() = "Sscanf" then i > 1 else i > 0 | output.isParameter(i))
     }
   }
+
+  /** The `Scan` function or one of its variants, all of which read from os.Stdin */
+  class Scanner extends Function {
+    Scanner() { this.hasQualifiedName("fmt", ["Scan", "Scanf", "Scanln"]) }
+  }
+
+  /** A call to a `Scanner`. */
+  class ScannerCall extends DataFlow::CallNode {
+    ScannerCall() { this.getTarget() instanceof Scanner }
+  }
+
+  /**
+   * The `Fscan` function or one of its variants,
+   * all of which read from a specified io.Reader
+   */
+  class FScanner extends Function {
+    FScanner() { this.hasQualifiedName("fmt", ["Fscan", "Fscanf", "Fscanln"]) }
+  }
+
+  /** A call to a `FScanner`. */
+  class FScannerCall extends DataFlow::CallNode {
+    FScannerCall() { this.getTarget() instanceof FScanner }
+
+    /**
+     * Returns the node corresponding to the io.Reader
+     * argument provided in the call.
+     */
+    DataFlow::Node getReader() { result = this.getArgument(0) }
+  }
 }
 
 /** Provides models of commonly used functions in the `io` package. */

ql/test/experimental/CWE-352/ConstantOauth2State.expected

@@ -0,0 +1,35 @@
+edges
+| ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:48:26:48:41 | stateStringConst |
+| ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:145:26:145:41 | stateStringConst |
+| ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:167:26:167:41 | stateStringConst |
+| ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:189:26:189:41 | stateStringConst |
+| ConstantOauth2State.go:20:22:20:28 | "state" : string | ConstantOauth2State.go:63:26:63:39 | stateStringVar |
+| ConstantOauth2State.go:78:11:78:25 | call to newFixedState : string | ConstantOauth2State.go:79:26:79:30 | state |
+| ConstantOauth2State.go:84:9:84:15 | "state" : string | ConstantOauth2State.go:78:11:78:25 | call to newFixedState : string |
+| ConstantOauth2State.go:145:9:145:42 | call to AuthCodeURL : string | ConstantOauth2State.go:146:54:146:56 | url |
+| ConstantOauth2State.go:167:9:167:42 | call to AuthCodeURL : string | ConstantOauth2State.go:168:54:168:56 | url |
+| ConstantOauth2State.go:189:9:189:42 | call to AuthCodeURL : string | ConstantOauth2State.go:190:28:190:30 | url |
+nodes
+| ConstantOauth2State.go:18:26:18:32 | "state" : string literal | semmle.label | "state" : string literal |
+| ConstantOauth2State.go:20:22:20:28 | "state" : string | semmle.label | "state" : string |
+| ConstantOauth2State.go:33:26:33:32 | "state" | semmle.label | "state" |
+| ConstantOauth2State.go:48:26:48:41 | stateStringConst | semmle.label | stateStringConst |
+| ConstantOauth2State.go:63:26:63:39 | stateStringVar | semmle.label | stateStringVar |
+| ConstantOauth2State.go:78:11:78:25 | call to newFixedState : string | semmle.label | call to newFixedState : string |
+| ConstantOauth2State.go:79:26:79:30 | state | semmle.label | state |
+| ConstantOauth2State.go:84:9:84:15 | "state" : string | semmle.label | "state" : string |
+| ConstantOauth2State.go:145:9:145:42 | call to AuthCodeURL : string | semmle.label | call to AuthCodeURL : string |
+| ConstantOauth2State.go:145:26:145:41 | stateStringConst | semmle.label | stateStringConst |
+| ConstantOauth2State.go:146:54:146:56 | url | semmle.label | url |
+| ConstantOauth2State.go:167:9:167:42 | call to AuthCodeURL : string | semmle.label | call to AuthCodeURL : string |
+| ConstantOauth2State.go:167:26:167:41 | stateStringConst | semmle.label | stateStringConst |
+| ConstantOauth2State.go:168:54:168:56 | url | semmle.label | url |
+| ConstantOauth2State.go:189:9:189:42 | call to AuthCodeURL : string | semmle.label | call to AuthCodeURL : string |
+| ConstantOauth2State.go:189:26:189:41 | stateStringConst | semmle.label | stateStringConst |
+| ConstantOauth2State.go:190:28:190:30 | url | semmle.label | url |
+#select
+| ConstantOauth2State.go:33:26:33:32 | "state" | ConstantOauth2State.go:33:26:33:32 | "state" | ConstantOauth2State.go:33:26:33:32 | "state" | Using a constant $@ to create oauth2 URLs. | ConstantOauth2State.go:33:26:33:32 | "state" | state string |
+| ConstantOauth2State.go:48:26:48:41 | stateStringConst | ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:48:26:48:41 | stateStringConst | Using a constant $@ to create oauth2 URLs. | ConstantOauth2State.go:18:26:18:32 | "state" | state string |
+| ConstantOauth2State.go:63:26:63:39 | stateStringVar | ConstantOauth2State.go:20:22:20:28 | "state" : string | ConstantOauth2State.go:63:26:63:39 | stateStringVar | Using a constant $@ to create oauth2 URLs. | ConstantOauth2State.go:20:22:20:28 | "state" | state string |
+| ConstantOauth2State.go:79:26:79:30 | state | ConstantOauth2State.go:84:9:84:15 | "state" : string | ConstantOauth2State.go:79:26:79:30 | state | Using a constant $@ to create oauth2 URLs. | ConstantOauth2State.go:84:9:84:15 | "state" | state string |
+| ConstantOauth2State.go:189:26:189:41 | stateStringConst | ConstantOauth2State.go:18:26:18:32 | "state" : string literal | ConstantOauth2State.go:189:26:189:41 | stateStringConst | Using a constant $@ to create oauth2 URLs. | ConstantOauth2State.go:18:26:18:32 | "state" | state string |