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

net/url: panic in QueryEscape #38643

Closed
tomdeering-wf opened this issue Apr 24, 2020 · 8 comments
Closed

net/url: panic in QueryEscape #38643

tomdeering-wf opened this issue Apr 24, 2020 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@tomdeering-wf
Copy link

tomdeering-wf commented Apr 24, 2020

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

$ go version
go version go1.13.10 linux/amd64

Does this issue reproduce with the latest release?

Unknown. I wish I knew a reproducible test case to try.

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

Binary built with:

go env Output
$ go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build206971722=/tmp/go-build -gno-record-gcc-switches"

Deployed in Docker container running busybox-1.31.1-r9

What did you do?

Our application code called the url.QueryEscape function with some (unfortunately unknown) input string.

What did you expect to see?

Encoded string returned to application code.

What did you see instead?

Panic runtime error: index out of range [59802754] with length 59802754 with the following stack:

	/usr/local/go/src/runtime/panic.go:679 +0x1b2
net/url.escape(0xc000318300, 0x1519bc0, 0x6, 0x0, 0x5)
	/usr/local/go/src/net/url/url.go:328 +0x42a
net/url.QueryEscape(...)
	/usr/local/go/src/net/url/url.go:273
net/url.Values.Encode(0xc000d30db0, 0x4, 0x6)
	/usr/local/go/src/net/url/url.go:930 +0x507
	... application code
@ianlancetaylor ianlancetaylor changed the title Panic in url.QueryEscape net/url: panic in QueryEscape Apr 24, 2020
@ianlancetaylor
Copy link
Contributor

Can you show us the output starting from the very beginning? There is some information missing, and it matters. You can omit the application code from the stack trace, that is probably not useful. Thanks.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 24, 2020
@networkimprov
Copy link

Fuzzing QueryEscape might surface a reproducer, if that hasn't been tried...

@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@odeke-em
Copy link
Member

@tomdeering-wf please take a look at Ian's request in #38643 (comment), the whole trace helps us out and currently we can't act on this issue.

@networkimprov great idea, indeed that seems like the sensible way to find out, would you like to handle that?

@networkimprov
Copy link

Tried this with 1.13.3 on linux/amd64, saw no panic. Maybe memory corruption?

package main

import (
   "net/url"
   "fmt"
)

func main() {
   fmt.Println("4 byte")
   for a := 0; a <= 255; a++ {
      if (a+1) % 8 == 0 { fmt.Println(a) }
      for b := 0; b <= 255; b++ {
      for c := 0; c <= 255; c++ {
      for d := 0; d <= 255; d++ {
         _ = url.QueryEscape(string([]byte{byte(a),byte(b),byte(c),byte(d)}))
      }}}
   }
}

@tomdeering-wf
Copy link
Author

tomdeering-wf commented May 26, 2020

Hi folks, sorry for the delay. Unfortunately I've lost the context beyond what is captured in this issue. We saw this happen just once in one of our cloud environments, and I also am unable to repro locally.

I thought I'd captured all the pertinent info from the stack, sorry Ian. If I'm able to relocate that panic, I'll post back with more info.

@tomdeering-wf
Copy link
Author

@ianlancetaylor found it. So, the stack above that was application code catching the panic and re-panicking. The subject of the panic was runtime error: index out of range [59802754] with length 59802754, which seems to have come out of the line t[j+2] = "0123456789ABCDEF"[c&15]. You're right, that does seem very relevant, sorry for the omission 🤦‍♂️

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 26, 2020
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone May 26, 2020
@ianlancetaylor
Copy link
Contributor

Thanks. Are you parsing URL values that are 59802754 bytes long?

From looking at the code I don't see how this panic could happen other than memory corruption, but I may be missing something.

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 31, 2022
@golang golang locked and limited conversation to collaborators Dec 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants