golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.31k stars 17.38k forks source link

text/template: reject whitespace as delimiter #55336

Closed catenacyber closed 1 year ago

catenacyber commented 1 year ago

What version of Go are you using (go version)?

$ go version
go version go1.17.6 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/catena/Library/Caches/go-build"
GOENV="/Users/catena/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/catena/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/catena/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.6"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/catena/go/src/github.com/catenacyber/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pp/dc1dtf9x2js3v0jx_m010nqr0000gn/T/go-build4237848497=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.17.6 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.17.6
uname -v: Darwin Kernel Version 21.3.0: Wed Jan  5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64
ProductName:    macOS
ProductVersion: 12.2.1
BuildVersion:   21D62
lldb --version: lldb-1316.0.9.41
Apple Swift version 5.6 (swiftlang-5.6.0.323.62 clang-1316.0.20.8)
gdb --version: GNU gdb (GDB) 9.1

What did you do?

Run https://go.dev/play/p/XRz1BdFwz4O?v=gotip

What did you expect to see?

The program finishing and printing somme dummy data

What did you see instead?

panic: runtime error: slice bounds out of range [:7] with length 5 [recovered]
    panic: runtime error: slice bounds out of range [:7] with length 5

goroutine 1 [running]:
text/template/parse.(*Tree).recover(0xc000070240, 0xc000051cd0)
    /usr/local/go-faketime/src/text/template/parse/parse.go:210 +0x15e
panic({0x4f17e0, 0xc00001a018})
    /usr/local/go-faketime/src/runtime/panic.go:884 +0x213
text/template/parse.(*lexer).thisItem(...)
    /usr/local/go-faketime/src/text/template/parse/lex.go:172
text/template/parse.lexRightDelim(0xc000068500)
    /usr/local/go-faketime/src/text/template/parse/lex.go:378 +0x2f2
text/template/parse.(*lexer).nextItem(0xc000068500)
    /usr/local/go-faketime/src/text/template/parse/lex.go:233 +0xe2
text/template/parse.(*Tree).peek(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:106 +0xb8
text/template/parse.(*Tree).operand(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:726 +0x69
text/template/parse.(*Tree).command(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:692 +0x105
text/template/parse.(*Tree).pipeline(0xc000070240, {0x4fbbd9, 0x7}, 0x10)
    /usr/local/go-faketime/src/text/template/parse/parse.go:502 +0x859
text/template/parse.(*Tree).action(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:418 +0x37a
text/template/parse.(*Tree).textOrAction(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:374 +0x2bc
text/template/parse.(*Tree).parse(0xc000070240)
    /usr/local/go-faketime/src/text/template/parse/parse.go:315 +0x309
text/template/parse.(*Tree).Parse(0xc000070240, {0x4fb555, 0x5}, {0x4fb16a?, 0xc0000a4d70?}, {0x4fb278?, 0x10?}, 0xc0000164e0, {0xc000014120, 0x2, ...})
    /usr/local/go-faketime/src/text/template/parse/parse.go:251 +0x535
text/template/parse.Parse({0x0, 0x0}, {0x4fb555, 0x5}, {0x4fb16a, 0x1}, {0x4fb278, 0x3}, {0xc000014120, 0x2, ...})
    /usr/local/go-faketime/src/text/template/parse/parse.go:66 +0x130
text/template.(*Template).Parse(0xc00001e0c0, {0x4fb555, 0x5})
    /usr/local/go-faketime/src/text/template/template.go:210 +0x7ca
html/template.(*Template).Parse(0xc000016480, {0x4fb555, 0x5})
    /usr/local/go-faketime/src/html/template/template.go:191 +0x90
main.main()
    /tmp/sandbox52285879/prog.go:13 +0x168

Program exited.

Found by https://github.com/catenacyber/ngolo-fuzzing on oss-fuzz https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51463

I think this was meant to be fixed by https://github.com/golang/go/issues/53261 cc @robpike

robpike commented 1 year ago

It has nothing to do with that fix, it's just a bug. But the program is pathological, probably generated by fuzzing. Using a space as a delimiter is never going to work.

The right fix, if it's worth doing anything, is probably to restrict the character set for delims.

seankhliao commented 1 year ago

wasn't this fixed for #52527

catenacyber commented 1 year ago

I thought it was as #53261 was merged cf https://github.com/golang/go/issues/52527#issuecomment-1149430888

But it reappeared shortly after

robpike commented 1 year ago

I want to be clear that this program is not sensible and no effort should be spent to make it succeed. I believe the right fix is to reject the delimiters, whether or not the actual out-of-range bug is fixed. You're using spaces (actually the "ignore spaces" character sequence) as a delimiter, and that is going to confuse the lexer irreparably. The best this program could do, if it didn't reject the delimiters outright, is explain in the documentation that the behavior is unpredictable.

The problem becomes whether we can restrict the character set for delimiters without breaking existing code, or just document that doing this kind of thing won't work.

robpike commented 1 year ago

I thought it was as #53261 was merged cf #52527 (comment)

But it reappeared shortly after

Removing the concurrency did not address any indexing problems, it just cleaned up the interaction with the parser and sped it up. That said, it should make them easier to fix, but see my other comment. I don't want this fixed.

gopherbot commented 1 year ago

Change https://go.dev/cl/433036 mentions this issue: text/template/parse: fix confusion about markers near right delims

rsc commented 1 year ago

A less line-noisy example is template.New("").Delims("{{- ", " -}}").Parse("{{- x -}}"). Yes it's dumb, but it should probably work. I sent a trivial CL.