Go: memory leakage queries

go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go

@@ -0,0 +1,32 @@
+package main
+
+import (
+	"fmt"
+	"os"
+)
+
+// BAD: defer Close inside a loop leaks file descriptors
+func badReadFiles(paths []string) {
+	for _, path := range paths {
+		f, err := os.Open(path)
+		if err != nil {
+			continue
+		}
+		defer f.Close() // not closed until function returns
+		fmt.Println(f.Name())
+	}
+}
+
+// GOOD: extract into a function so defer runs per iteration
+func goodReadFiles(paths []string) {
+	for _, path := range paths {
+		func() {
+			f, err := os.Open(path)
+			if err != nil {
+				return
+			}
+			defer f.Close() // closed at end of this closure
+			fmt.Println(f.Name())
+		}()
+	}
+}

go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+In Go, <code>defer</code> schedules a function call to run when the <em>enclosing function</em>
+returns — not when the enclosing block or loop iteration ends. Deferring a resource release
+call (such as <code>Close</code>, <code>Unlock</code>, or <code>Rollback</code>) inside a loop
+means that cleanup calls accumulate and only execute after the loop finishes and the function
+returns.
+</p>
+
+<p>
+This can lead to resource exhaustion: file descriptors pile up, database connections are held
+open, locks are held longer than intended, or transactions remain open across iterations.
+</p>
+
+</overview>
+<recommendation>
+<p>
+Extract the loop body into a separate function or closure so that <code>defer</code> runs
+at the end of each iteration:
+</p>
+
+<sample src="DeferReleaseInLoop.go" />
+
+<p>
+Alternatively, call the cleanup function directly without <code>defer</code> at the
+appropriate point in the loop body.
+</p>
+
+</recommendation>
+<references>
+<li>
+    <a href="https://go.dev/ref/spec#Defer_statements">Go Language Specification — Defer statements</a>
+</li>
+<li>
+    <a href="https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01">Gotchas of Defer in Go</a>
+</li>
+</references>
+</qhelp>

go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql

@@ -0,0 +1,48 @@
+/**
+ * @name Deferred resource release in loop
+ * @id tob/go/defer-release-in-loop
+ * @description Deferring a resource release (Close, Rollback, etc.) inside a loop delays cleanup until the enclosing function returns, causing resource leaks across iterations such as file descriptor exhaustion or connection pool starvation.
+ * @kind problem
+ * @tags security
+ * @problem.severity warning
+ * @precision medium
+ * @security-severity 3.0
+ * @group security
+ */
+
+import go
+import ResourceModel
+
+/**
+ * Holds if `inner` is a (transitive) child of `outer` without crossing
+ * a function literal boundary. This ensures we don't catch defers inside
+ * closures that are invoked per-iteration.
+ */
+predicate parentWithoutFuncLit(AstNode inner, AstNode outer) {
+  inner.getParent() = outer and not inner instanceof FuncLit
+  or
+  exists(AstNode mid |
+    parentWithoutFuncLit(inner, mid) and
+    parentWithoutFuncLit(mid, outer)
+  )
+}
+
+/**
+ * A `defer` statement that defers a resource close/release call.
+ */
+class DeferredResourceClose extends DeferStmt {
+  ResourceCloseMethod closeMethod;
+
+  DeferredResourceClose() { closeMethod = this.getCall().getTarget() }
+
+  string getMethodName() { result = closeMethod.getName() }
+}
+
+from DeferredResourceClose deferStmt, LoopStmt loop
+where
+  parentWithoutFuncLit(deferStmt, loop.(ForStmt).getBody())
+  or
+  parentWithoutFuncLit(deferStmt, loop.(RangeStmt).getBody())
+select deferStmt,
+  "Deferred " + deferStmt.getMethodName() +
+    "() in a loop will not execute until the function returns, leaking resources across iterations."

go/src/security/DeferReleaseInLoop/ResourceModel.qll

@@ -0,0 +1,71 @@
+/**
+ * Models for file, network, and database resource acquisition and release in Go.
+ *
+ * These models identify:
+ * - "Source" functions that return closeable resources (files, connections, etc.)
+ * - "Sink" methods that release/close those resources
+ */
+
+import go
+
+/**
+ * A function that opens or acquires a file-based resource. No common trait used
+ */
+class FileResourceAcquisition extends Function {
+  FileResourceAcquisition() {
+    // os package: file open/create
+    this.hasQualifiedName("os", ["Open", "OpenFile", "Create", "CreateTemp", "NewFile", "Pipe"])
+    or
+    // net package: connections and listeners
+    this.hasQualifiedName("net", ["Dial", "DialTimeout", "Listen", "ListenPacket"])
+    or
+    this.hasQualifiedName("net", ["FileConn", "FileListener", "FilePacketConn"])
+    or
+    this.(Method).hasQualifiedName("net", "Dialer", ["Dial", "DialContext"])
+    or
+    // net/http: client requests returning response bodies
+    this.hasQualifiedName("net/http", ["Get", "Post", "PostForm", "Head"])
+    or
+    this.(Method).hasQualifiedName("net/http", "Client", ["Do", "Get", "Post", "PostForm", "Head"])
+    or
+    // crypto/tls
+    this.hasQualifiedName("crypto/tls", ["Dial", "DialWithDialer", "Client", "Server"])
+    or
+    this.(Method).hasQualifiedName("crypto/tls", "Dialer", "DialContext")
+    or
+    // compress readers/writers (implement io.Closer)
+    this.hasQualifiedName("compress/gzip", ["NewReader", "NewWriter", "NewWriterLevel"])
+    or
+    this.hasQualifiedName("compress/zlib",
+      ["NewReader", "NewWriter", "NewWriterLevel", "NewWriterLevelDict"])
+    or
+    this.hasQualifiedName("compress/flate", ["NewReader", "NewWriter"])
+    or
+    this.hasQualifiedName("compress/lzw", ["NewReader", "NewWriter"])
+    or
+    // archive readers
+    this.hasQualifiedName("archive/zip", "OpenReader")
+  }
+}
+
+/**
+ * A method that closes or releases a file-based resource that shouldn't be leaked.
+ * Matching on those that implement `io.Closer` for now.
+ */
+class ResourceCloseMethod extends Method {
+  ResourceCloseMethod() {
+    this.getName() = "Close" and
+    (
+      this.getReceiverType().implements("io", "Closer")
+      or
+      exists(Type base |
+        base = this.getReceiverType().(PointerType).getBaseType()
+        or
+        not this.getReceiverType() instanceof PointerType and
+        base = this.getReceiverType()
+      |
+        base.implements("io", "Closer")
+      )
+    )
+  }
+}

go/src/security/UnboundedIORead/UnboundedIORead.go

@@ -0,0 +1,19 @@
+package main
+
+import (
+	"io"
+	"net/http"
+)
+
+// BAD: unbounded read of request body
+func badHandler(w http.ResponseWriter, r *http.Request) {
+	body, _ := io.ReadAll(r.Body) // no size limit — OOM on large request
+	w.Write(body)
+}
+
+// GOOD: limit body size before reading
+func goodHandler(w http.ResponseWriter, r *http.Request) {
+	r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MB limit
+	body, _ := io.ReadAll(r.Body)
+	w.Write(body)
+}

go/src/security/UnboundedIORead/UnboundedIORead.qhelp

@@ -0,0 +1,35 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+Reading an HTTP request body with <code>io.ReadAll</code> (or the deprecated
+<code>ioutil.ReadAll</code>) allocates the entire body into memory with no upper bound.
+A malicious client can send an arbitrarily large request body to exhaust server memory,
+causing a denial-of-service condition.
+</p>
+
+</overview>
+<recommendation>
+<p>
+Wrap the request body with a size-limiting reader before reading it:
+</p>
+
+<sample src="UnboundedIORead.go" />
+
+<p>
+Prefer <code>http.MaxBytesReader</code> which also sets the appropriate error on the
+response, or <code>io.LimitReader</code> for non-HTTP contexts.
+</p>
+
+</recommendation>
+<references>
+<li>
+    <a href="https://pkg.go.dev/net/http#MaxBytesReader">http.MaxBytesReader documentation</a>
+</li>
+<li>
+    <a href="https://pkg.go.dev/io#LimitReader">io.LimitReader documentation</a>
+</li>
+</references>
+</qhelp>

go/src/security/UnboundedIORead/UnboundedIORead.ql

@@ -0,0 +1,101 @@
+/**
+ * @name Unbounded read of request body
+ * @id tob/go/unbounded-io-read
+ * @description Reading an HTTP request body with `io.ReadAll` or `ioutil.ReadAll` without a size limit allows a malicious client to exhaust server memory with an arbitrarily large request.
+ * @kind path-problem
+ * @tags security
+ * @problem.severity error
+ * @precision high
+ * @security-severity 7.5
+ * @group security
+ */
+
+import go
+
+/**
+ * An `io.ReadAll` or `ioutil.ReadAll` call — functions that read an entire
+ * reader into memory with no size bound.
+ */
+class UnboundedReadCall extends DataFlow::CallNode {
+  UnboundedReadCall() {
+    this.getTarget().hasQualifiedName("io", "ReadAll")
+    or
+    this.getTarget().hasQualifiedName("io/ioutil", "ReadAll")
+    or
+    this.getTarget().hasQualifiedName("ioutil", "ReadAll")
+  }
+}
+
+/**
+ * A source node: an HTTP request body that can be controlled by an attacker.
+ *
+ * Matches `r.Body` where `r` is an `*http.Request`.
+ */
+class HTTPRequestBodySource extends DataFlow::Node {
+  HTTPRequestBodySource() {
+    exists(Field f, SelectorExpr sel |
+      f.hasQualifiedName("net/http", "Request", "Body") and
+      sel.getSelector().refersTo(f) and
+      this.asExpr() = sel
+    )
+  }
+}
+
+/**
+ * A call that wraps a reader with a size limit, acting as a sanitizer.
+ */
+class SizeLimitingCall extends DataFlow::CallNode {
+  SizeLimitingCall() {
+    this.getTarget().hasQualifiedName("net/http", "MaxBytesReader")
+    or
+    this.getTarget().hasQualifiedName("io", "LimitReader")
+  }
+}
+
+/**
+ * Holds if `r.Body` is reassigned from a size-limiting call in the same function
+ * that `bodyRead` is in, on the same request variable `r`.
+ */
+predicate bodyLimitedByReassignment(SelectorExpr bodyRead) {
+  exists(SizeLimitingCall limiter, Assignment assign, SelectorExpr lhs, Variable v |
+    // The assignment target is `v.Body`
+    assign.getLhs() = lhs and
+    lhs.getSelector().getName() = "Body" and
+    lhs.getBase().(Ident).refersTo(v) and
+    // The RHS is a MaxBytesReader/LimitReader call
+    assign.getRhs() = limiter.asExpr() and
+    // The body read is on the same variable: `v.Body`
+    bodyRead.getBase().(Ident).refersTo(v) and
+    // Both in the same function
+    assign.getEnclosingFunction() = bodyRead.getEnclosingFunction()
+  )
+}
+
+module UnboundedReadConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source instanceof HTTPRequestBodySource and
+    // Exclude r.Body reads where r.Body was reassigned from a size limiter
+    not bodyLimitedByReassignment(source.asExpr())
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(UnboundedReadCall readAll | sink = readAll.getArgument(0))
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    // If the body passes through a size-limiting wrapper (io.LimitReader pattern), it is safe
+    node = any(SizeLimitingCall c).getResult(0)
+    or
+    node = any(SizeLimitingCall c).getResult()
+  }
+}
+
+module UnboundedReadFlow = DataFlow::Global<UnboundedReadConfig>;
+
+import UnboundedReadFlow::PathGraph
+
+from UnboundedReadFlow::PathNode source, UnboundedReadFlow::PathNode sink
+where UnboundedReadFlow::flowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Unbounded read of HTTP request $@ with no size limit allows denial of service via memory exhaustion.",
+  source.getNode(), "body"

go/test/query-tests/security/DeferReleaseInLoop/DeferReleaseInLoop.expected

@@ -0,0 +1,8 @@
+| DeferReleaseInLoop.go:19:3:19:17 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:31:3:31:17 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:43:3:43:25 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:55:3:55:20 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:67:3:67:18 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:84:3:84:18 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:85:3:85:17 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |
+| DeferReleaseInLoop.go:98:4:98:18 | defer statement | Deferred Close() in a loop will not execute until the function returns, leaking resources across iterations. |