Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Go: memory leakage queries by ex0dus-0x · Pull Request #39 · trailofbits/codeql-queries · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/QUERIES.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,9 +44,11 @@

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[Deferred resource release in loop](../go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md)|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.|warning|medium|
|[Invalid file permission parameter](../go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
|[Missing MinVersion in tls.Config](../go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
|[Trim functions misuse](../go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
|[Unbounded read of request body](../go/src/docs/security/UnboundedIORead/UnboundedIORead.md)|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.|error|high|

### Java and Kotlin

Expand Down
51 changes: 51 additions & 0 deletions go/src/docs/security/DeferReleaseInLoop/DeferReleaseInLoop.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
# Deferred resource release in loop
In Go, `defer` schedules a function call to run when the *enclosing function* returns — not when the enclosing block or loop iteration ends. Deferring a resource release call (such as `Close`, `Unlock`, or `Rollback`) inside a loop means that cleanup calls accumulate and only execute after the loop finishes and the function returns.

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.


## Recommendation
Extract the loop body into a separate function or closure so that `defer` runs at the end of each iteration:


```go
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())
}()
}
}

```
Alternatively, call the cleanup function directly without `defer` at the appropriate point in the loop body.


## References
* [Go Language Specification — Defer statements](https://go.dev/ref/spec#Defer_statements)
* [Gotchas of Defer in Go](https://blog.learngoprogramming.com/gotchas-of-defer-in-go-1-8d070894cb01)
36 changes: 36 additions & 0 deletions go/src/docs/security/UnboundedIORead/UnboundedIORead.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
# Unbounded read of request body
Reading an HTTP request body with `io.ReadAll` (or the deprecated `ioutil.ReadAll`) 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.


## Recommendation
Wrap the request body with a size-limiting reader before reading it:


```go
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)
}

```
Prefer `http.MaxBytesReader` which also sets the appropriate error on the response, or `io.LimitReader` for non-HTTP contexts.


## References
* [http.MaxBytesReader documentation](https://pkg.go.dev/net/http#MaxBytesReader)
* [io.LimitReader documentation](https://pkg.go.dev/io#LimitReader)
72 changes: 72 additions & 0 deletions go/src/ext/ResourceAcquisitions.model.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
# Models-as-Data definitions of functions that acquire a resource which must
# later be released (Close, etc.). Consumed by DeferReleaseInLoop.ql via
# `sourceNode(node, "tob-resource-acq")`.
#
# The `output` column selects which acquired value to mark:
# - "ReturnValue" -> the first return value (the common `(resource, error)` shape).
# - "ReturnValue[n]" -> the n-th return value, for functions that return more
# than one resource (e.g. `os.Pipe` returns a read and a
# write `*os.File`, so both ends are modeled below).
#
# Row format (sourceModel):
# [package, type, subtypes, name, signature, ext, output, kind, provenance]
extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
# os
- ["os", "", false, "Open", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "OpenFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Create", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "CreateTemp", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "NewFile", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# os.Pipe returns (r, w *File, err): model both the read end and write end.
- ["os", "", false, "Pipe", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["os", "", false, "Pipe", "", "", "ReturnValue[1]", "tob-resource-acq", "manual"]
# net (package-level)
- ["net", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "DialTimeout", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "Listen", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "ListenPacket", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FileListener", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "", false, "FilePacketConn", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net.Dialer (method)
- ["net", "Dialer", true, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http (package-level)
- ["net/http", "", false, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "", false, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# net/http.Client (method)
- ["net/http", "Client", true, "Do", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Get", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Post", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "PostForm", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["net/http", "Client", true, "Head", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls (package-level)
- ["crypto/tls", "", false, "Dial", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "DialWithDialer", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Client", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["crypto/tls", "", false, "Server", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# crypto/tls.Dialer (method)
- ["crypto/tls", "Dialer", true, "DialContext", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/gzip
- ["compress/gzip", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/gzip", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/zlib
- ["compress/zlib", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevel", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/zlib", "", false, "NewWriterLevelDict", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/flate
- ["compress/flate", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/flate", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# compress/lzw
- ["compress/lzw", "", false, "NewReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
- ["compress/lzw", "", false, "NewWriter", "", "", "ReturnValue", "tob-resource-acq", "manual"]
# archive/zip
- ["archive/zip", "", false, "OpenReader", "", "", "ReturnValue", "tob-resource-acq", "manual"]
2 changes: 2 additions & 0 deletions go/src/qlpack.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,5 +8,7 @@ library: false
extractor: go
dependencies:
codeql/go-all: "*"
dataExtensions:
- ext/**/*.model.yml
suites: codeql-suites
defaultSuiteFile: codeql-suites/tob-go-code-scanning.qls
32 changes: 32 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.go
Original file line numberDiff line numberDiff line change
@@ -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())
}()
}
}
42 changes: 42 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
68 changes: 68 additions & 0 deletions go/src/security/DeferReleaseInLoop/DeferReleaseInLoop.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
/**
* @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 semmle.go.dataflow.ExternalFlow

/**
* Holds if `inner` is a (transitive) child of `outer` without crossing
* a function literal boundary.
*/
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)
)
}

/** Holds if `node` is inside the body of `loop`, not crossing closures. */
predicate inLoopBody(AstNode node, LoopStmt loop) {
parentWithoutFuncLit(node, loop.(ForStmt).getBody())
or
parentWithoutFuncLit(node, loop.(RangeStmt).getBody())
}

/**
* Gets the root identifier of an identifier or (possibly nested) selector
* expression: `f` for `f`, `resp` for `resp.Body`, `a` for `a.b.c`.
*/
Ident selectorRoot(Expr e) {
result = e
or
result = selectorRoot(e.(SelectorExpr).getBase())
}

/**
* Gets the data-flow node for the root variable of a `defer x.Close()`
* receiver chain. For `defer f.Close()`, this is `f`; for
* `defer resp.Body.Close()`, this is `resp`; for `defer a.b.c.Close()`,
* this is `a`.
*/
DataFlow::Node deferCloseReceiverBase(DeferStmt d) {
d.getCall().getTarget().getName() = "Close" and
result.asExpr() = selectorRoot(d.getCall().getCalleeExpr().(SelectorExpr).getBase())
}

from DeferStmt deferStmt, DataFlow::CallNode acquisition, DataFlow::Node acquired, LoopStmt loop
where
// Match any return value modeled as a resource acquisition, not just the
// first — e.g. `os.Pipe` returns both a read and a write `*os.File`.
acquired = acquisition.getResult(_) and
sourceNode(acquired, "tob-resource-acq") and
inLoopBody(acquisition.asExpr(), loop) and
inLoopBody(deferStmt, loop) and
DataFlow::localFlow(acquired, deferCloseReceiverBase(deferStmt))
select deferStmt,
"Deferred Close() of resource acquired from $@ in a loop will not execute until the function returns, leaking resources across iterations.",
acquisition, acquisition.getTarget().getName() + "()"
19 changes: 19 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.go
Original file line numberDiff line numberDiff line change
@@ -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)
}
35 changes: 35 additions & 0 deletions go/src/security/UnboundedIORead/UnboundedIORead.qhelp
Original file line numberDiff line numberDiff line change
@@ -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>
Loading
Loading