Go: Add Cors Gin Support

go/ql/lib/change-notes/2023-10-31-add-gin-cors-framework.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added the [gin cors](https://github.com/gin-contrib/cors) library to the CorsMisconfiguration.ql query

go/ql/lib/go.qll

@@ -41,6 +41,7 @@ import semmle.go.frameworks.Email
 import semmle.go.frameworks.Encoding
 import semmle.go.frameworks.Fiber
 import semmle.go.frameworks.Gin
+import semmle.go.frameworks.GinCors
 import semmle.go.frameworks.Glog
 import semmle.go.frameworks.GoKit
 import semmle.go.frameworks.GoMicro

go/ql/lib/semmle/go/frameworks/GinCors.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides classes for modeling the `github.com/gin-contrib/cors` package.
+ */
+
+import go
+
+/**
+ * Provides classes for modeling the `github.com/gin-contrib/cors` package.
+ */
+module GinCors {
+  /** Gets the package name `github.com/gin-gonic/gin`. */
+  string packagePath() { result = package("github.com/gin-contrib/cors", "") }
+
+  /**
+   * A new function create a new gin Handler that passed to gin as middleware
+   */
+  class New extends Function {
+    New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) }
+  }
+
+  /**
+   * A write to the value of Access-Control-Allow-Credentials header
+   */
+  class AllowCredentialsWrite extends DataFlow::ExprNode {
+    DataFlow::Node base;
+    GinConfig gc;
+
+    AllowCredentialsWrite() {
+      exists(Field f, Write w |
+        f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
+        w.writesField(base, f, this) and
+        this.getType() instanceof BoolType and
+        (
+          gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
+            base.asInstruction() or
+          gc.getV().getAUse() = base
+        )
+      )
+    }
+
+    /**
+     * Get config variable holding header values
+     */
+    GinConfig getConfig() { result = gc }
+  }
+
+  /**
+   * A write to the value of Access-Control-Allow-Origins header
+   */
+  class AllowOriginsWrite extends DataFlow::ExprNode {
+    DataFlow::Node base;
+    GinConfig gc;
+
+    AllowOriginsWrite() {
+      exists(Field f, Write w |
+        f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
+        w.writesField(base, f, this) and
+        this.asExpr() instanceof SliceLit and
+        (
+          gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
+            base.asInstruction() or
+          gc.getV().getAUse() = base
+        )
+      )
+    }
+
+    /**
+     * Get config variable holding header values
+     */
+    GinConfig getConfig() { result = gc }
+  }
+
+  /**
+   * A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins
+   */
+  class AllowAllOriginsWrite extends DataFlow::ExprNode {
+    DataFlow::Node base;
+    GinConfig gc;
+
+    AllowAllOriginsWrite() {
+      exists(Field f, Write w |
+        f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
+        w.writesField(base, f, this) and
+        this.getType() instanceof BoolType and
+        (
+          gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
+            base.asInstruction() or
+          gc.getV().getAUse() = base
+        )
+      )
+    }
+
+    /**
+     * Get config variable holding header values
+     */
+    GinConfig getConfig() { result = gc }
+  }
+
+  /**
+   * A variable of type Config that holds the headers to be set.
+   */
+  class GinConfig extends Variable {
+    SsaWithFields v;
+
+    GinConfig() {
+      this = v.getBaseVariable().getSourceVariable() and
+      exists(Type t | t.hasQualifiedName(packagePath(), "Config") | v.getType() = t)
+    }
+
+    /**
+     * Get variable declaration of GinConfig
+     */
+    SsaWithFields getV() { result = v }
+  }
+}

go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql

@@ -69,22 +69,48 @@ module UntrustedToAllowOriginHeaderConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { isSinkHW(sink, _) }
 }
 
+module UntrustedToAllowOriginConfigConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
+
+  additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) { sink = w }
+
+  predicate isSink(DataFlow::Node sink) { isSinkWrite(sink, _) }
+}
+
 /**
  * Tracks taint flowfor reasoning about when an `UntrustedFlowSource` flows to
  * a `HeaderWrite` that writes an `Access-Control-Allow-Origin` header's value.
  */
 module UntrustedToAllowOriginHeaderFlow = TaintTracking::Global<UntrustedToAllowOriginHeaderConfig>;
 
+/**
+ * Tracks taint flowfor reasoning about when an `UntrustedFlowSource` flows to
+ * a `AllowOriginsWrite` that writes an `Access-Control-Allow-Origin` header's value.
+ */
+module UntrustedToAllowOriginConfigFlow = TaintTracking::Global<UntrustedToAllowOriginConfigConfig>;
+
 /**
  * Holds if the provided `allowOriginHW` HeaderWrite's parent ResponseWriter
  * also has another HeaderWrite that sets a `Access-Control-Allow-Credentials`
  * header to `true`.
  */
-predicate allowCredentialsIsSetToTrue(AllowOriginHeaderWrite allowOriginHW) {
+predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOriginHW) {
   exists(AllowCredentialsHeaderWrite allowCredentialsHW |
     allowCredentialsHW.getHeaderValue().toLowerCase() = "true"
   |
-    allowOriginHW.getResponseWriter() = allowCredentialsHW.getResponseWriter()
+    allowOriginHW.(AllowOriginHeaderWrite).getResponseWriter() =
+      allowCredentialsHW.getResponseWriter()
+  )
+  or
+  exists(GinCors::AllowCredentialsWrite allowCredentialsGin |
+    allowCredentialsGin.toString() = "true"
+  |
+    //flow only goes in one direction so fix this before PR
+    allowCredentialsGin.getConfig() = allowOriginHW.(GinCors::AllowOriginsWrite).getConfig() and
+    not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
+      allowAllOrigins.toString() = "true" and
+      allowCredentialsGin.getConfig() = allowAllOrigins.getConfig()
+    )
   )
 }
 
@@ -93,10 +119,13 @@ predicate allowCredentialsIsSetToTrue(AllowOriginHeaderWrite allowOriginHW) {
  * UntrustedFlowSource.
  * The `message` parameter is populated with the warning message to be returned by the query.
  */
-predicate flowsFromUntrustedToAllowOrigin(AllowOriginHeaderWrite allowOriginHW, string message) {
+predicate flowsFromUntrustedToAllowOrigin(DataFlow::ExprNode allowOriginHW, string message) {
   exists(DataFlow::Node sink |
     UntrustedToAllowOriginHeaderFlow::flowTo(sink) and
     UntrustedToAllowOriginHeaderConfig::isSinkHW(sink, allowOriginHW)
+    or
+    UntrustedToAllowOriginConfigFlow::flowTo(sink) and
+    UntrustedToAllowOriginConfigConfig::isSinkWrite(sink, allowOriginHW)
   |
     message =
       headerAllowOrigin() + " header is set to a user-defined value, and " +
@@ -108,11 +137,23 @@ predicate flowsFromUntrustedToAllowOrigin(AllowOriginHeaderWrite allowOriginHW,
  * Holds if the provided `allowOriginHW` HeaderWrite is for a `Access-Control-Allow-Origin`
  * header and the value is set to `null`.
  */
-predicate allowOriginIsNull(AllowOriginHeaderWrite allowOriginHW, string message) {
-  allowOriginHW.getHeaderValue().toLowerCase() = "null" and
+predicate allowOriginIsNull(DataFlow::ExprNode allowOriginHW, string message) {
+  allowOriginHW.(AllowOriginHeaderWrite).getHeaderValue().toLowerCase() = "null" and
+  message =
+    headerAllowOrigin() + " header is set to `" +
+      allowOriginHW.(AllowOriginHeaderWrite).getHeaderValue() + "`, and " + headerAllowCredentials()
+      + " is set to `true`"
+  or
+  allowOriginHW
+      .(GinCors::AllowOriginsWrite)
+      .asExpr()
+      .(SliceLit)
+      .getAnElement()
+      .toString()
+      .toLowerCase() = "\"null\"" and
   message =
-    headerAllowOrigin() + " header is set to `" + allowOriginHW.getHeaderValue() + "`, and " +
-      headerAllowCredentials() + " is set to `true`"
+    headerAllowOrigin() + " header is set to `" + "null" + "`, and " + headerAllowCredentials() +
+      " is set to `true`"
 }
 
 /**
@@ -170,15 +211,15 @@ module FromUntrustedFlow = TaintTracking::Global<FromUntrustedConfig>;
 /**
  * Holds if the provided `allowOriginHW` is also destination of a `UntrustedFlowSource`.
  */
-predicate flowsToGuardedByCheckOnUntrusted(AllowOriginHeaderWrite allowOriginHW) {
+predicate flowsToGuardedByCheckOnUntrusted(DataFlow::ExprNode allowOriginHW) {
   exists(DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn |
     FromUntrustedFlow::flowTo(sink) and FromUntrustedConfig::isSinkCgn(sink, cgn)
   |
     cgn.dominates(allowOriginHW.getBasicBlock())
   )
 }
 
-from AllowOriginHeaderWrite allowOriginHW, string message
+from DataFlow::ExprNode allowOriginHW, string message
 where
   allowCredentialsIsSetToTrue(allowOriginHW) and
   (

go/ql/test/experimental/CWE-942/CorsGin.go

@@ -0,0 +1,85 @@
+package main
+
+import (
+	"net/http"
+	"time"
+
+	"github.com/gin-contrib/cors"
+	"github.com/gin-gonic/gin"
+)
+
+/*
+** Function is vulnerable due to AllowAllOrigins = true aka Access-Control-Allow-Origin: null
+ */
+func vunlnerable() {
+	router := gin.Default()
+	// CORS for https://foo.com and null
+	// - PUT and PATCH methods
+	// - Origin header
+	// - Credentials share
+	// - Preflight requests cached for 12 hours
+	config_vulnerable := cors.Config{
+		AllowMethods:     []string{"PUT", "PATCH"},
+		AllowHeaders:     []string{"Origin"},
+		ExposeHeaders:    []string{"Content-Length"},
+		AllowCredentials: true,
+		MaxAge:           12 * time.Hour,
+	}
+	config_vulnerable.AllowOrigins = []string{"null", "https://foo.com"}
+	router.Use(cors.New(config_vulnerable))
+	router.GET("/", func(c *gin.Context) {
+		c.String(http.StatusOK, "hello world")
+	})
+	router.Run()
+}
+
+/*
+** Function is safe due to hardcoded origin and AllowCredentials: true
+ */
+func safe() {
+	router := gin.Default()
+	// CORS for https://foo.com origin, allowing:
+	// - PUT and PATCH methods
+	// - Origin header
+	// - Credentials share
+	// - Preflight requests cached for 12 hours
+	config_safe := cors.Config{
+		AllowMethods:     []string{"PUT", "PATCH"},
+		AllowHeaders:     []string{"Origin"},
+		ExposeHeaders:    []string{"Content-Length"},
+		AllowCredentials: true,
+		MaxAge:           12 * time.Hour,
+	}
+	config_safe.AllowOrigins = []string{"https://foo.com"}
+	router.Use(cors.New(config_safe))
+	router.GET("/", func(c *gin.Context) {
+		c.String(http.StatusOK, "hello world")
+	})
+	router.Run()
+}
+
+/*
+** Function is safe due to AllowAllOrigins = true aka Access-Control-Allow-Origin: *
+ */
+func AllowAllTrue() {
+	router := gin.Default()
+	// CORS for "*" origin, allowing:
+	// - PUT and PATCH methods
+	// - Origin header
+	// - Credentials share
+	// - Preflight requests cached for 12 hours
+	config_allowall := cors.Config{
+		AllowMethods:     []string{"PUT", "PATCH"},
+		AllowHeaders:     []string{"Origin"},
+		ExposeHeaders:    []string{"Content-Length"},
+		AllowCredentials: true,
+		MaxAge:           12 * time.Hour,
+	}
+	config_allowall.AllowOrigins = []string{"null"}
+	config_allowall.AllowAllOrigins = true
+	router.Use(cors.New(config_allowall))
+	router.GET("/", func(c *gin.Context) {
+		c.String(http.StatusOK, "hello world")
+	})
+	router.Run()
+}

go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected

@@ -1,3 +1,4 @@
+| CorsGin.go:28:35:28:69 | slice literal | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
 | CorsMisconfiguration.go:26:4:26:56 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
 | CorsMisconfiguration.go:32:4:32:42 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
 | CorsMisconfiguration.go:53:4:53:44 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |

go/ql/test/experimental/CWE-942/CorsMisconfiguration.go

@@ -191,9 +191,9 @@ func main() {
 		// })
 		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
 			// OK-ish: the input origin header is validated against a whitelist.
-			if origin := req.Header.Get("Origin"); cors[origin] {
+			if origin := req.Header.Get("Origin"); cors_map[origin] {
 				w.Header().Set("Access-Control-Allow-Origin", origin)
-			} else if len(origin) > 0 && cors["*"] {
+			} else if len(origin) > 0 && cors_map["*"] {
 				w.Header().Set("Access-Control-Allow-Origin", origin)
 			}
 
@@ -219,7 +219,7 @@ func main() {
 }
 
 var (
-	cors = map[string]bool{"*": true}
+	cors_map = map[string]bool{"*": true}
 )
 
 func GetAllowOrigin() []string {

go/ql/test/experimental/CWE-942/go.mod

@@ -0,0 +1,35 @@
+module corsmiconfiguration/test
+
+go 1.21
+
+require (
+	github.com/gin-contrib/cors v1.4.0
+	github.com/gin-gonic/gin v1.9.1
+)
+
+require (
+	github.com/bytedance/sonic v1.9.1 // indirect
+	github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect
+	github.com/gabriel-vasile/mimetype v1.4.2 // indirect
+	github.com/gin-contrib/sse v0.1.0 // indirect
+	github.com/go-playground/locales v0.14.1 // indirect
+	github.com/go-playground/universal-translator v0.18.1 // indirect
+	github.com/go-playground/validator/v10 v10.14.0 // indirect
+	github.com/goccy/go-json v0.10.2 // indirect
+	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/klauspost/cpuid/v2 v2.2.4 // indirect
+	github.com/leodido/go-urn v1.2.4 // indirect
+	github.com/mattn/go-isatty v0.0.19 // indirect
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.2 // indirect
+	github.com/pelletier/go-toml/v2 v2.0.8 // indirect
+	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
+	github.com/ugorji/go/codec v1.2.11 // indirect
+	golang.org/x/arch v0.3.0 // indirect
+	golang.org/x/crypto v0.9.0 // indirect
+	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/sys v0.8.0 // indirect
+	golang.org/x/text v0.9.0 // indirect
+	google.golang.org/protobuf v1.30.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)