Merge branch 'main' into workflow/coverage/update

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

@@ -81,18 +81,28 @@ module SQL {
                   "github.com/lann/squirrel"
                 ], "")
           |
-            // first argument to `squirrel.Expr`
-            fn.hasQualifiedName(sq, "Expr")
+            fn.hasQualifiedName(sq, ["Delete", "Expr", "Insert", "Select", "Update"])
             or
-            // first argument to the `Prefix`, `Suffix` or `Where` method of one of the `*Builder` classes
-            exists(string builder | builder.matches("%Builder") |
-              fn.(Method).hasQualifiedName(sq, builder, "Prefix") or
-              fn.(Method).hasQualifiedName(sq, builder, "Suffix") or
-              fn.(Method).hasQualifiedName(sq, builder, "Where")
+            exists(Method m, string builder | m = fn |
+              builder = ["DeleteBuilder", "InsertBuilder", "SelectBuilder", "UpdateBuilder"] and
+              m.hasQualifiedName(sq, builder,
+                ["Columns", "From", "Options", "OrderBy", "Prefix", "Suffix", "Where"])
+              or
+              builder = "InsertBuilder" and
+              m.hasQualifiedName(sq, builder, ["Replace", "Into"])
+              or
+              builder = "SelectBuilder" and
+              m.hasQualifiedName(sq, builder,
+                ["CrossJoin", "GroupBy", "InnerJoin", "LeftJoin", "RightJoin"])
+              or
+              builder = "UpdateBuilder" and
+              m.hasQualifiedName(sq, builder, ["Set", "Table"])
             )
           ) and
-          this = fn.getACall().getArgument(0) and
-          this.getType().getUnderlyingType() instanceof StringType
+          this = fn.getACall().getArgument(0)
+        |
+          this.getType().getUnderlyingType() instanceof StringType or
+          this.getType().getUnderlyingType().(SliceType).getElementType() instanceof StringType
         )
       }
     }

go/ql/src/change-notes/2024-03-05-squirrel-sqli-sinks.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `go/sql-injection` now recognizes more sinks in the package `github.com/Masterminds/squirrel`.

go/ql/src/experimental/CWE-770/DenialOfService.qhelp

@@ -0,0 +1,32 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+
+<qhelp>
+  <overview>
+    <p>Using untrusted input to created with the built-in make function 
+      could lead to excessive memory allocation and potentially cause the program to crash due 
+      to running out of memory. This vulnerability could be exploited to perform a DoS attack by consuming all available server resources.</p>
+  </overview>
+
+  <recommendation>
+    <p>Implement a maximum allowed value for creates a slice with the built-in make function to prevent excessively large allocations. 
+    For instance, you could restrict it to a reasonable upper limit.</p>
+  </recommendation>
+
+  <example>
+    <p>In the following example snippet, the <code>n</code> field is user-controlled.</p>
+    <p> The server trusts that n has an acceptable value, however when using a maliciously large value, 
+      it allocates a slice of <code>n</code> of strings before filling the slice with data.</p>
+
+    <sample src="DenialOfServiceBad.go" />
+
+    <p>One way to prevent this vulnerability is by implementing a maximum allowed value for the user-controlled input:</p>
+
+    <sample src="DenialOfServiceGood.go" />
+  </example>
+
+  <references>
+    <li>
+      OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Denial_of_Service_Cheat_Sheet.html">Denial of Service Cheat Sheet</a>
+    </li>
+  </references>
+</qhelp>

go/ql/src/experimental/CWE-770/DenialOfService.ql

@@ -0,0 +1,59 @@
+/**
+ * @name Denial Of Service
+ * @description slices created with the built-in make function from user-controlled sources using a
+ * maliciously large value possibly leading to a denial of service.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9
+ * @precision high
+ * @id go/denial-of-service
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-770
+ */
+
+import go
+
+/**
+ * Holds if the guard `g` on its branch `branch` checks that `e` is not constant and is less than some other value.
+ */
+predicate denialOfServiceSanitizerGuard(DataFlow::Node g, Expr e, boolean branch) {
+  exists(DataFlow::Node lesser |
+    e = lesser.asExpr() and
+    g.(DataFlow::RelationalComparisonNode).leq(branch, lesser, _, _) and
+    not e.isConst()
+  )
+}
+
+/**
+ * Module for defining predicates and tracking taint flow related to denial of service issues.
+ */
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(Function f, DataFlow::CallNode cn | cn = f.getACall() |
+      f.hasQualifiedName("strconv", ["Atoi", "ParseInt", "ParseUint", "ParseFloat"]) and
+      node1 = cn.getArgument(0) and
+      node2 = cn.getResult(0)
+    )
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node = DataFlow::BarrierGuard<denialOfServiceSanitizerGuard/3>::getABarrierNode()
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink = Builtin::make().getACall().getArgument(0) }
+}
+
+/**
+ * Tracks taint flow for reasoning about denial of service, where source is
+ * user-controlled and unchecked.
+ */
+module Flow = TaintTracking::Global<Config>;
+
+import Flow::PathGraph
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
+select sink, source, sink, "This variable might be leading to denial of service."

go/ql/src/experimental/CWE-770/DenialOfServiceBad.go

@@ -0,0 +1,27 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+)
+
+func OutOfMemoryBad(w http.ResponseWriter, r *http.Request) {
+	query := r.URL.Query()
+
+	queryStr := query.Get("n")
+	collectionSize, err := strconv.Atoi(queryStr)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	result := make([]string, collectionSize)
+	for i := 0; i < collectionSize; i++ {
+		result[i] = fmt.Sprintf("Item %d", i+1)
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(result)
+}

go/ql/src/experimental/CWE-770/DenialOfServiceGood.go

@@ -0,0 +1,30 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+)
+
+func OutOfMemoryGood(w http.ResponseWriter, r *http.Request) {
+	query := r.URL.Query()
+	MaxValue := 6
+	queryStr := query.Get("n")
+	collectionSize, err := strconv.Atoi(queryStr)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+	if collectionSize < 0 || collectionSize > MaxValue {
+		http.Error(w, "Bad request", http.StatusBadRequest)
+		return
+	}
+	result := make([]string, collectionSize)
+	for i := 0; i < collectionSize; i++ {
+		result[i] = fmt.Sprintf("Item %d", i+1)
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(result)
+}

go/ql/test/experimental/CWE-770/DenialOfService.expected

@@ -0,0 +1,18 @@
+edges
+| DenialOfServiceBad.go:11:12:11:16 | selection of URL | DenialOfServiceBad.go:11:12:11:24 | call to Query | provenance |  |
+| DenialOfServiceBad.go:11:12:11:24 | call to Query | DenialOfServiceBad.go:13:15:13:20 | source | provenance |  |
+| DenialOfServiceBad.go:13:15:13:20 | source | DenialOfServiceBad.go:13:15:13:29 | call to Get | provenance |  |
+| DenialOfServiceBad.go:13:15:13:29 | call to Get | DenialOfServiceBad.go:14:28:14:36 | sourceStr | provenance |  |
+| DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | DenialOfServiceBad.go:20:27:20:30 | sink | provenance |  |
+| DenialOfServiceBad.go:14:28:14:36 | sourceStr | DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | provenance |  |
+nodes
+| DenialOfServiceBad.go:11:12:11:16 | selection of URL | semmle.label | selection of URL |
+| DenialOfServiceBad.go:11:12:11:24 | call to Query | semmle.label | call to Query |
+| DenialOfServiceBad.go:13:15:13:20 | source | semmle.label | source |
+| DenialOfServiceBad.go:13:15:13:29 | call to Get | semmle.label | call to Get |
+| DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | semmle.label | ... := ...[0] |
+| DenialOfServiceBad.go:14:28:14:36 | sourceStr | semmle.label | sourceStr |
+| DenialOfServiceBad.go:20:27:20:30 | sink | semmle.label | sink |
+subpaths
+#select
+| DenialOfServiceBad.go:20:27:20:30 | sink | DenialOfServiceBad.go:11:12:11:16 | selection of URL | DenialOfServiceBad.go:20:27:20:30 | sink | This variable might be leading to denial of service. |

go/ql/test/experimental/CWE-770/DenialOfService.qlref

@@ -0,0 +1 @@
+experimental/CWE-770/DenialOfService.ql

go/ql/test/experimental/CWE-770/DenialOfServiceBad.go

@@ -0,0 +1,27 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+)
+
+func OutOfMemoryBad(w http.ResponseWriter, r *http.Request) {
+	source := r.URL.Query()
+
+	sourceStr := source.Get("n")
+	sink, err := strconv.Atoi(sourceStr)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	result := make([]string, sink)
+	for i := 0; i < sink; i++ {
+		result[i] = fmt.Sprintf("Item %d", i+1)
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(result)
+}

go/ql/test/experimental/CWE-770/DenialOfServiceGood.go

@@ -0,0 +1,94 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+)
+
+func OutOfMemoryGood1(w http.ResponseWriter, r *http.Request) {
+	source := r.URL.Query()
+	MaxValue := 6
+	sourceStr := source.Get("n")
+	sink, err := strconv.Atoi(sourceStr)
+	if err != nil || sink < 0 {
+		http.Error(w, "Bad request", http.StatusBadRequest)
+		return
+	}
+	if sink > MaxValue {
+		return
+	}
+	result := make([]string, sink)
+	for i := 0; i < sink; i++ {
+		result[i] = fmt.Sprintf("Item %d", i+1)
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(result)
+}
+
+func OutOfMemoryGood2(w http.ResponseWriter, r *http.Request) {
+	source := r.URL.Query()
+	MaxValue := 6
+	sourceStr := source.Get("n")
+	sink, err := strconv.Atoi(sourceStr)
+	if err != nil || sink < 0 {
+		http.Error(w, "Bad request", http.StatusBadRequest)
+		return
+	}
+	if sink <= MaxValue {
+		result := make([]string, sink)
+		for i := 0; i < sink; i++ {
+			result[i] = fmt.Sprintf("Item %d", i+1)
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(result)
+	}
+}
+
+func OutOfMemoryGood3(w http.ResponseWriter, r *http.Request) {
+	source := r.URL.Query()
+	MaxValue := 6
+	sourceStr := source.Get("n")
+	sink, err := strconv.Atoi(sourceStr)
+	if err != nil || sink < 0 {
+		http.Error(w, "Bad request", http.StatusBadRequest)
+		return
+	}
+	if sink > MaxValue {
+		sink = MaxValue
+		result := make([]string, sink)
+		for i := 0; i < sink; i++ {
+			result[i] = fmt.Sprintf("Item %d", i+1)
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(result)
+	}
+}
+
+func OutOfMemoryGood4(w http.ResponseWriter, r *http.Request) {
+	source := r.URL.Query()
+	MaxValue := 6
+	sourceStr := source.Get("n")
+	sink, err := strconv.Atoi(sourceStr)
+	if err != nil || sink < 0 {
+		http.Error(w, "Bad request", http.StatusBadRequest)
+		return
+	}
+	if sink > MaxValue {
+		sink = MaxValue
+	} else {
+		tmp := sink
+		sink = tmp
+	}
+	result := make([]string, sink)
+	for i := 0; i < sink; i++ {
+		result[i] = fmt.Sprintf("Item %d", i+1)
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(result)
+}

go/ql/test/library-tests/semmle/go/frameworks/SQL/go.mod

@@ -3,15 +3,21 @@ module semmle.go.frameworks.SQL
 go 1.13
 
 require (
-	github.com/Masterminds/squirrel v1.1.0
+	github.com/Masterminds/squirrel v1.5.4
 	github.com/go-pg/pg v8.0.6+incompatible
 	github.com/go-pg/pg/v9 v9.1.3
 	github.com/go-sql-driver/mysql v1.6.0 // indirect
 	github.com/go-xorm/xorm v0.7.9
 	github.com/kr/text v0.2.0 // indirect
 	github.com/lib/pq v1.10.2 // indirect
-	github.com/uptrace/bun v1.1.14
-	github.com/uptrace/bun/dialect/sqlitedialect v1.1.14
-	github.com/uptrace/bun/driver/sqliteshim v1.1.14
+	github.com/mattn/go-isatty v0.0.19 // indirect
+	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
+	github.com/stretchr/testify v1.8.1 // indirect
+	golang.org/x/tools v0.9.1 // indirect
+	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
+	lukechampine.com/uint128 v1.3.0 // indirect
+	modernc.org/libc v1.22.6 // indirect
+	modernc.org/sqlite v1.22.1 // indirect
+	modernc.org/token v1.1.0 // indirect
 	xorm.io/xorm v1.1.0
 )