Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text/template: consider adding recursion depth limit for deeply nested expressions #71201

Open
thevilledev opened this issue Jan 9, 2025 · 1 comment

Comments

@thevilledev
Copy link

Go version

go version go1.23.4 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/ville/Library/Caches/go-build'
GOENV='/Users/ville/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/ville/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/ville/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.4/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.4/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.4'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/ville/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/cl/npk3dq855kxf2ns3qth9pv4m0000gn/T/go-build3530549792=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Note: This is a public issue after discussing with the Go security team.

Created a program to test template parsing with deeply nested parentheses: https://go.dev/play/p/659Ry2YDb4Z

package main

import (
    "fmt"
    "runtime"
    "strings"
    "text/template"
)

func main() {
    depth := 1000
    expr := fmt.Sprintf("{{$x := %s1+2i%s}}",
        strings.Repeat("(", depth),
        strings.Repeat(")", depth))

    fmt.Printf("Input size: %d bytes\n", len(expr))

    var m1 runtime.MemStats
    runtime.ReadMemStats(&m1)

    _, err := template.New("test").Parse(expr)
    if err != nil {
        fmt.Printf("Error: %v\n", err)
    }

    var m2 runtime.MemStats
    runtime.ReadMemStats(&m2)
    allocated := m2.TotalAlloc - m1.TotalAlloc

    fmt.Printf("Memory allocated: %d bytes\n", allocated)
    fmt.Printf("Amplification factor: %.2fx\n", float64(allocated)/float64(len(expr)))
}

What did you see happen?

  1. With moderate nesting (1000 levels), we observe significant memory amplification:
Input size: 2014 bytes
Memory allocated: 172256 bytes
Amplification factor: 85.53x
  1. Attempting deeper nesting (tested with depth = 500000) causes a stack overflow before reaching extreme memory allocation:
runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0x14020580340 stack=[0x14020580000, 0x14040580000]
fatal error: stack overflow

What did you expect to see?

Two improvements would be helpful:

  1. A reasonable limit on expression nesting depth to prevent accidental stack overflow. Open to help with the implementation. Example:
const maxParenDepth = 100

type Tree struct {
    // ... existing fields ...
    parenDepth int
}

func (t *Tree) term() Node {
    case itemLeftParen:
        if t.parenDepth >= maxParenDepth {
            t.errorf("expression too deeply nested (max %d)", maxParenDepth)
        }
        t.parenDepth++
        defer func() { t.parenDepth-- }()
        // ... rest of implementation
}
  1. Documentation clarity: While html/template documentation explicitly states "The security model used by this package assumes that template authors are trusted" in its package documentation, text/template lacks similar guidance. Adding this documentation would help users better understand the package's security model.

Both changes would align with common parser implementation practices while maintaining clarity about the trust model.

@gabyhelp
Copy link

gabyhelp commented Jan 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants