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

Jump to bottom

net: call net.ResolveIPAddr with nul char causes a panic in the goroutine on Windows build #31597

Closed
saito-tom opened this issue Apr 21, 2019 · 4 comments
Closed

net: call net.ResolveIPAddr with nul char causes a panic in the goroutine on Windows build #31597

saito-tom opened this issue Apr 21, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.13

Comments

@saito-tom
Copy link

saito-tom commented Apr 21, 2019 •
edited

What did you do?

My product's server crashed and reproduced the problem from the panic log.
The reproduction program is below.
https://play.golang.org/p/1FdRbei_83s

The reproduction program does not work on Go Playground. Create a windows build and run it.

What did you expect to see?

I wanted to return an error if an invalid string is passed.

addr:  127.0.0.1
err: lookup localhost: no such host

What did you see instead?

Output of Windows build binary (GOOS=windows go run resolve_ip.go)

0032:fixme:process:SetProcessPriorityBoost (0xffffffffffffffff,1): stub
addr:  127.0.0.1
panic: syscall: string with NUL passed to StringToUTF16

goroutine 6 [running]:
syscall.StringToUTF16(...)
        /home/st/Downloads/go/src/syscall/syscall_windows.go:30
syscall.StringToUTF16Ptr(0x4f03b7, 0xa, 0x0)
        /home/st/Downloads/go/src/syscall/syscall_windows.go:65 +0x8b
net.(*Resolver).lookupIP.func1(0x0, 0x0, 0x0, 0x0, 0x0)
        /home/st/Downloads/go/src/net/lookup_windows.go:99 +0xf1
net.(*Resolver).lookupIP.func2(0xc000004100, 0xc00003c240)
        /home/st/Downloads/go/src/net/lookup_windows.go:129 +0x32
created by net.(*Resolver).lookupIP
        /home/st/Downloads/go/src/net/lookup_windows.go:128 +0xf8
exit status 2

Output of Linux build binary (go run resolve_ip.go)

addr:  127.0.0.1
err: lookup localhost: no such host

System details

go version go1.12.4 linux/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/st/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/st/go"
GOPROXY=""
GORACE=""
GOROOT="/home/st/Downloads/go"
GOTMPDIR=""
GOTOOLDIR="/home/st/Downloads/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
uname -sr: Linux 4.19.32-1-MANJARO
LSB Version:	n/a
Distributor ID:	ManjaroLinux
Description:	Manjaro Linux
Release:	18.0.4
Codename:	Illyria
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.28.
gdb --version: GNU gdb (GDB) 8.2.1
@saito-tom
Copy link
Author

saito-tom commented Apr 21, 2019 •
edited

In lookupIP, it seems like it is not good to call Deprecated syscall.StringToUTF16Ptr which causes panic.

lookupIP:

go/src/net/lookup_windows.go

Line 99 in 68d4b12

e := syscall.GetAddrInfoW(syscall.StringToUTF16Ptr(name), nil, &hints, &result)

syscall.StringToUTF16Ptr:

go/src/syscall/syscall_windows.go

Lines 65 to 66 in 68d4b12

// Deprecated: Use UTF16PtrFromString instead.
func StringToUTF16Ptr(s string) *uint16 { return &StringToUTF16(s)[0] }

go/src/syscall/syscall_windows.go

Lines 27 to 33 in 68d4b12

// Deprecated: Use UTF16FromString instead.
func StringToUTF16(s string) []uint16 {
a, err := UTF16FromString(s)
if err != nil {
panic("syscall: string with NUL passed to StringToUTF16")
}
return a

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 22, 2019
@julieqiu julieqiu added this to the Go1.13 milestone Apr 22, 2019
@julieqiu
Copy link
Member

/cc @mikioh @bradfitz

@gopherbot
Copy link

Change https://golang.org/cl/173362 mentions this issue: net: don't crash on Windows when Lookup name has null byte in string

@gopherbot
Copy link

Change https://golang.org/cl/217729 mentions this issue: net: don't check LookupHost error in TestLookupNullByte

@dmitshur dmitshur mentioned this issue Feb 5, 2020
Open
gopherbot pushed a commit that referenced this issue Feb 5, 2020
@dmitshur
net: don't check LookupHost error in TestLookupNullByte
fb93cd4 
net.LookupHost("foo\x00bar") may resolve successfully on some networks.
Reduce the scope of the test to check only that the call doesn't panic.

Also update the test comment to reference the relevant issue.

Fixes #37031
Updates #31597

Change-Id: If175deed8121625ef507598c6145e937ccffd89e
Reviewed-on: https://go-review.googlesource.com/c/go/+/217729
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Feb 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
3 participants
@julieqiu @gopherbot @saito-tom