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.

Sign up for GitHub

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

Jump to bottom

text/template: reject whitespace as delimiter #55336

Closed
catenacyber opened this issue Sep 22, 2022 · 7 comments
Closed

text/template: reject whitespace as delimiter #55336

catenacyber opened this issue Sep 22, 2022 · 7 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@catenacyber
Copy link
Contributor

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 #53261 cc @robpike
@robpike
Copy link
Contributor

robpike commented Sep 22, 2022

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.

@mvdan mvdan changed the title text/template: untime error: slice bounds out of range [:7] with length 5 text/template: slice bounds out of range [:7] with length 5 Sep 22, 2022
@seankhliao
Copy link
Member

wasn't this fixed for #52527

@catenacyber
Copy link
Contributor Author

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

But it reappeared shortly after

@robpike
Copy link
Contributor

robpike commented Sep 22, 2022 •
edited

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
Copy link
Contributor

robpike commented Sep 22, 2022

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.

@cherrymui cherrymui added this to the Unplanned milestone Sep 22, 2022
@cherrymui cherrymui added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Sep 22, 2022
@cherrymui cherrymui changed the title text/template: slice bounds out of range [:7] with length 5 text/template: reject whitespace as delimiter Sep 22, 2022
@gopherbot
Copy link

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

@rsc
Copy link
Contributor

rsc commented Sep 23, 2022

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.

@golang golang locked and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@rsc @robpike @gopherbot @seankhliao @cherrymui @catenacyber