Golang 代码审查笔记 II

# Golang 代码审查笔记 II - elttam 来源：https://www.elttam.com/blog/golang-code-review-notes-ii ## 引言 几年前，我们发布了一篇博客文章（https://www.elttam.com/blog/golang-codereview），旨在为代码审计人员和注重安全的工程师提供一份参考资料，方便他们在审计或开发 Golang 项目时查阅。在那篇文章中，我们介绍了在自己 Golang 代码审计项目中经常遇到的一些 bug 类别。 多亏了热力学第二定律，时间只能向前推进。作为一门得到良好支持的现代编程语言，Go 自我们第一篇博客文章以来经历了大量变化和改进。因此，我们决定以同样的思路撰写这篇后续文章。 自我们第一篇博文发布以来，自动化工具和 AI 助手提高了代码审查的基准：许多明显的 bug 现在在第一轮审查中就会被任何人发现。但这个基准也正是大多数审查者止步的地方，因为工具也止步于此——规则只会标记它们已知要寻找的内容。资深审计人员的与众不同之处在于，他们了解语言中尚未被任何规则覆盖的尖锐边缘，并有意识地主动寻找它们。 在这篇文章中，我们首先快速回顾一下语言发生了哪些变化，使得安全性对人们来说更容易、更直观（对于要涵盖的内容没有特别的筛选标准）。然后，我们会重点介绍一些新的“陷阱”，希望能让所有审计或开发 Golang 项目的人受益。最后，我们还将发布一些 semgrep 规则，以缩小这些危险编码模式带来的差距。这些规则可以在我们的 Semgrep 规则仓库（https://github.com/elttam/semgrep-rules）中找到。 引言到此结束，首先让我们看看 Golang 引入的一些对安全格局产生重大影响的功能。 ## 之前的情况 先从一件略微令人担忧的事情说起：我们上一篇博客文章中的几乎所有内容在 2026 年仍然有效——但有一个重要的例外。虽然路径遍历仍然是现实存在的威胁，但一年多前 Go 1.24 发布时引入了一套名为 `os.Root` 的新 API（https://go.dev/blog/osroot）。无需过多重复 Go 团队博客文章中已经解释得很清楚的内容，这些 API 旨在通过创建一个根目录，并在框架层面禁止读写超出此范围的内容，从而彻底阻止路径遍历。根目录以下的所有内容仍然可用，包括子目录和符号链接（只要它们保持在根目录内）。 虽然这是一个非常有用的功能，但在使用过程中也出现了一些问题（https://github.com/golang/go/issues/73555）（https://github.com/golang/go/issues/77827）；不过这些问题在 Go 1.25 中很快得到了修复。 类似地，Go 团队刚刚结束了他们的 `math/rand` 改进时代，该改进跨越了 Go 1.20 到 1.26（即本文发布时的当前版本）。Go 从“向 `rsa.GenerateKey` 传递 nil 会静默导致灾难性后果”转变为“无论你传递什么，该函数都会做出正确的事情”。这是通过清理 `math/rand` 实现的，开发人员现在可以在 PRNG 和 `crypto/rand` 之间做出明确选择——而手动种子生成器目前已经完全从库中移除。 此外，在生成私钥时意外使用不安全随机数的问题也通过确保这些函数忽略传递给它们的任何 `io.Reader` 并始终无条件使用安全的内部随机数而得到解决。 ## 陷阱 好了，铺垫够多了，现在开始深入探讨实际的陷阱。下面的小节重点介绍其中的几个，主要关注标准库功能。最近，Go 周围的工具链也因审查力度加大而有了相当多的安全改进，不过这些要等到本系列博文的第三部分。此外，我们目前也不会关注流行的第三方框架，只关注标准库功能。 ### 静默整数溢出 与大多数其他语言不同，Go 的 `int` 和 `uint` 类型没有固定的宽度——它们在编译时根据目标平台设定：在 amd64 和 arm64 上是 64 位，在 386、arm 和 WebAssembly 上是 32 位。这是一个设计选择，以便做出高效的大小选择；然而，由于 Go 可以交叉编译，这意味着除非开发人员小心谨慎，否则他们可能对整数的大小做出错误假设。Go 静默地允许整数溢出和下溢，不会引发任何运行时错误或恐慌，如果开发人员没有考虑到这一点，可能会引入细微且严重的安全漏洞。 例如，经典的字符串到整数转换函数 `strconv.Atoi` 返回一个普通的 `int`，这意味着它的范围静默地依赖于架构。虽然如果返回的 `int` 超出其范围，`strconv.Atoi` 会报错，但这仍然会在下游导致静默的环绕问题。 Paul Gerste 在 DEF CON 32 上的演讲《SQL 注入并未消亡：在协议层面走私查询》（https://media.defcon.org/DEF%20CON%2032/DEF%20CON%2032%20presentations/DEF%20CON%2032%20-%20Paul%20Gerste%20-%20SQL%20Injection%20Isn%27t%20Dead%20Smuggling%20Queries%20at%20the%20Protocol%20Level.pdf）展示了这个问题的实际影响。Gerste 在 Go 数据库驱动库中发现了多个整数溢出，这些溢出能够溢出相应有线协议的长度字段。通过溢出长度字段，他成功演示了在数据包发送到后端数据库时注入新的查询。 这类漏洞并不仅限于数据库驱动。整数溢出是 Go 数据序列化库中反复出现的问题，其中属性的长度来自不受信任的输入，可能导致数据结构边界的错误解释。 例如，`github.com/tinylib/msgp` 包（https://github.com/tinylib/msgp）实现了 MessagePack 二进制序列化格式（https://msgpack.org/index.html），其规范规定 `Binary`、`String`、`Array` 和 `Map` 对象的最大长度为 \(2^32\)-1 字节（https://github.com/msgpack/msgpack/blob/master/spec.md#limitation）。在 `github.com/tinylib/msgp` 中发现的一个错误是，他们使用 Go 的 `len` 函数（https://pkg.go.dev/builtin#len）来获取要序列化的数据的长度，然后使用 `uint32()` 进行强制类型转换，如下面的代码片段所示。 *`github.com/tinylib/[email protected]` - `msgp/write.go`* （https://github.com/tinylib/msgp/blob/v1.5.0/msgp/write.go#L491） ```go // WriteString writes a messagepack string to the writer. // (This is NOT an implementation of io.StringWriter) func (mw *Writer) WriteString(s string) error { sz := uint32(len(s)) <1> var err error switch { case sz <= 31: err = mw.push(wfixstr(uint8(sz))) case sz <= math.MaxUint8: err = mw.prefix8(mstr8, uint8(sz)) case sz <= math.MaxUint16: err = mw.prefix16(mstr16, uint16(sz)) default: err = mw.prefix32(mstr32, sz) } if err != nil { return err } return mw.writeString(s) <2> } ``` 1. 输入字符串的长度使用 `uint32()` 进行了强制类型转换。 2. 写入字符串的完整长度，而不受 `sz` 限制。 Go 的 `len` 函数返回一个 `int` 类型，其中 `int` 是一个有符号整数，至少为 32 位（https://pkg.go.dev/builtin#int），但在 64 位系统上，它返回一个 64 位有符号整数（https://go.dev/ref/spec#Numeric_types）。这使得长度字段可以溢出，从而在反序列化过程中操作数据结构的其他部分，如下面的代码和终端输出所示。 *使用 `github.com/tinylib/[email protected]` 的示例概念验证* ```go //go:generate msgp package main import ( "bytes" "fmt" "log" "strings" "github.com/tinylib/msgp/msgp" ) //msgp:tuple Example type Example struct { UserInput string NoUserInput string } func MsgPackDemo(userInput string) { original := Example{ UserInput: userInput, NoUserInput: "user should not be able to overwrite this", } // Serialise var buf bytes.Buffer w := msgp.NewWriter(&buf) if err := original.EncodeMsg(w); err != nil { log.Fatalf("failed to serialise: %v", err) } if err := w.Flush(); err != nil { log.Fatalf("failed to flush writer: %v", err) } fmt.Printf("original.NoUserInput: %s\n", original.NoUserInput) // Deserialise var deserialised Example r := msgp.NewReader(&buf) if err := deserialised.DecodeMsg(r); err != nil { log.Fatalf("failed to deserialise: %v", err) } fmt.Printf("Deserialised struct: %+v\n\n", deserialised) } func main() { fmt.Println("demo normal use") MsgPackDemo("normal user") fmt.Println("demo int overflow") // \xd9 corresponds to a string with an 8-bit length prefix // \x12 is the length of the following string overflowString := "\xd9\x12HACKER OVERWRITTEN" overflowAmount := (1 << 32) - len(overflowString) MsgPackDemo("hacker" + overflowString + strings.Repeat("A", overflowAmount)) } ``` *上述代码的输出* ``` $ go build && ./msgpoverflow demo normal use original.NoUserInput: user should not be able to overwrite this Deserialised struct: {UserInput:normal user NoUserInput:user should not be able to overwrite this} demo int overflow original.NoUserInput: user should not be able to overwrite this Deserialised struct: {UserInput:hacker NoUserInput:HACKER OVERWRITTEN} ``` 为了帮助解决静默整数溢出问题，Trail of Bits 发布了 `go-panikint`（https://blog.trailofbits.com/2025/12/31/detect-gos-silent-arithmetic-bugs-with-go-panikint/），这是一个开源工具，用于在运行时检测 Go 程序中的静默算术溢出。 ### ReverseProxy 移除头部 与互联网世界中的几乎所有事物一样，HTTP 头部由几个 RFC 定义——不同实现如何准确遵守这些 RFC 则是另一个话题。与我们当前兴趣相关的一个 RFC 是 RFC 2616（https://datatracker.ietf.org/doc/html/rfc2616#section-13.5.1），它定义了一个逐跳（hop-by-hop）头部列表。顾名思义，这些头部仅对单个连接有意义，因此不应由代理转发。顺便提一下，虽然这个 RFC 现在已过时，并被下面描述的 RFC 取代，但仍然有一些遗留代码遵守这个标准。 为了“简化”事情，有一个更新的 RFC——RFC 7230（https://datatracker.ietf.org/doc/html/rfc7230#section-6.1），它旨在取代 RFC 2616——定义了 `Connection:` 头部，该头部应指示下一跳接收方哪些头部是逐跳的，从而应在转发消息之前将其移除。 现在将其与我们的 Golang 冒险联系起来：Go 标准库中有一个反向代理，位于 `httputil.ReverseProxy` 下。这个反向代理非常“听话”，它尊重**两个** RFC，而这正是困惑的来源。该模块还有一个 `Director` 函数，可用于管理传入请求。这个陷阱在 GitHub 问题（https://github.com/golang/go/issues/50580）中有很好的解释，但我们在此直接引用： 1. 克隆传入请求以生成出站请求 outreq。 2. 将 outreq 传递给 Director 函数，该函数可能会修改它。 3. 从 outreq 中移除逐跳头部。 4. 将 outreq 发送到后端。 现在考虑以下代码： ```go proxy := &httputil.ReverseProxy{ Director: func(req *http.Request) { req.URL.Scheme = "http" req.URL.Host = "internal-backend:8080" // Director 添加此头部以向后端验证请求。 req.Header.Set("X-Internal-Auth", os.Getenv("BACKEND_SECRET")) // Director 还添加此头部以强制后端进行传输安全。 req.Header.Set("X-Forwarded-Proto", "https") }, } ``` 如果攻击者知道这些头部名称，并在恶意请求的 `Connection:` 头部中指定它们，他们可以强制丢弃认证或传输加密： ``` # 攻击者的请求——将 Director 添加的头部命名为逐跳： GET /admin HTTP/1.1 Host: proxy.example.com Connection: X-Internal-Auth X-Internal-Auth: anything ``` 虽然这绝对是一个边缘情况，但它真实存在，并且过去已经导致了安全公告：Ory Oathkeeper 中的逐跳滥用导致头部修改器被破坏（https://github.com/ory/oathkeeper/security/advisories/GHSA-w9mr-28mw-j8hg）。 好消息是，在 Go 1.26 中，`Director` 函数已被弃用，取而代之的是 `Rewrite` 函数，该函数在逐跳头部被移除*之后*接收请求，这使得它成为一个默认安全的选项： ```go proxy := &httputil.ReverseProxy{ Rewrite: func(req *httputil.ProxyRequest) { req.SetURL(&url.URL{Scheme: "http", Host: "internal-backend:8080"}) req.Out.Header.Set("X-Internal-Auth", os.Getenv("BACKEND_SECRET")) req.SetXForwarded() }, } ``` 然而，为了向后兼容，`Director` 并未移除，因此任何仍在使用它的项目仍然存在漏洞。 ### 修改 `net/url` 结构体 下一个陷阱很微妙。有时我们需要创建 `net/url` 结构体的副本，以便对 URL 进行某些处理，但又不想让原始结构体发生变化。因此我们可能会这样做： ```go var redirectURL, _ = url.Parse("https://auth.example.com/callback") func main() { http.ListenAndServe(":8080", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { u := redirectURL q := u.Query() token, err := getRedirectToken(r) if err != nil { q.Set("error", err.Error()) } else { q.Set("token", token) } u.RawQuery = q.Encode() http.Redirect(w, r, u.String(), http.StatusFound) })) } ``` 乍一看似乎没问题。但问题在于 `u := redirectURL` 并不是我们期望的结构体副本，我们只是复制了一个**指针**。因此，当我们尝试写入“副本”时，例如 `u.RawQuery = q.Encode()`，我们实际上是在写入原始结构体！这是一个竞态条件，可能导致安全问题。我们未能找到任何公开披露的由这种模式直接导致的漏洞，可能是因为以下原因： - 竞态窗口可能非常窄。 - Go 工具链的 `-race` 标志在拥有良好的测试套件的情况下应该能捕获此类问题。 - 一个易受攻击的应用程序需要将 `url` 结构体存储在共享状态中*并且*同时对其进行修改，这是在陷阱之上的开发人员错误。 然而，我们仍想包含这种模式，因为它在 Go 中仍然是一个活跃的陷阱。一种安全的做法是解引用指针以复制实际结构体，例如：`u := *redirectURL`。或者创建一个只复制相关字段的新结构体： ```go u := &url.URL{ Scheme: redirectURL.Scheme, Host: redirectURL.Host, Path: redirectURL.Path, } ``` ### 空字节认证绕过 Go 字符串是长度分隔的字节切片。与 C 不同，没有空终止符，也没有任何字符是特殊的——Go 字符串可以在任何位置包含 `\x00`，语言会正确处理。（顺便提一下，如果你曾经在没有工具的情况下查看过编译后的 Go 二进制文件并试图查找字符串，并且没有崩溃，我向你致敬。）因此 `len("admin\x00evil")` 将返回 12，而 `"admin\x00evil" == "admin"` 将为 false。 这个陷阱出现在 Go 和使用 C 风格空终止字符串的任何东西之间的边界上：CGO、PAM、LDAP 客户端、带有 C 层的数据库驱动程序，或者任何接受路径或凭证的系统调用。在这个边界上，字符串会在第一个空字节处静默截断，而 Go 层毫不知情。 考虑以下经典的身份验证逻辑： ```go // 应用程序在 Go 层阻止直接 admin 登录， // 然后将实际的凭据验证委托给基于 C 的身份验证库。 func login(username, password string) (bool, error) { // 黑名单：阻止以