Skip to content

x/sys/windows: NTUnicodeString of PROCESS_BASIC_INFORMATION.PebBaseAddress.ProcessParameters.CommandLine is incorrectly converted to slice #73460

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

Closed
xformerfhs opened this issue Apr 21, 2025 · 6 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@xformerfhs
Copy link

xformerfhs commented Apr 21, 2025

Go version

go version go1.24.2 windows/amd64

Output of go env in your module/workspace:

set AR=ar
set CC=gcc
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_ENABLED=0
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set CXX=g++
set GCCGO=gccgo
set GO111MODULE=
set GOAMD64=v3
set GOARCH=amd64
set GOAUTH=netrc
set GOBIN=
set GOCACHE=C:\Users\User\AppData\Local\go-build
set GOCACHEPROG=
set GODEBUG=
set GOENV=C:\Users\User\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFIPS140=off
set GOFLAGS=
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=T:\UserTemp\User\go-build1636068885=/tmp/go-build -gno-record-gcc-switches
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMOD=D:\Users\User\go\pkg\mod\golang.org\x\s[email protected]\go.mod
set GOMODCACHE=D:\Users\User\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\Users\User\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTELEMETRY=on
set GOTELEMETRYDIR=C:\Users\User\AppData\Roaming\go\telemetry
set GOTMPDIR=T:\UserTemp\User
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.24.2
set GOWORK=
set PKG_CONFIG=pkg-config

What did you do?

I wanted to write a function that wipes the command line in the Windows Process Environment Block so that sensible information like password or key parameters do not show up when processes are listed.

The package golang.org/x/sys/windows has all that is needed to accomplish this.

Getting at the command line in the PEB is easily done with this code:

var (
	ntDLL                   = windows.MustLoadDLL(`ntdll.dll`)
	queryInformationProcess = ntDLL.MustFindProc(`NtQueryInformationProcess`)
)

func WipeCommandLine() error {
	hProcess, err := windows.OpenProcess(
		windows.PROCESS_QUERY_INFORMATION|windows.PROCESS_VM_READ,
		false,
		windows.GetCurrentProcessId(),
	)
	if err != nil {
		return err
	}

	var pbi windows.PROCESS_BASIC_INFORMATION
	var status uintptr
	status, _, _ = queryInformationProcess.Call(
		uintptr(hProcess),
		windows.ProcessBasicInformation,
		uintptr(unsafe.Pointer(&pbi)),
		unsafe.Sizeof(pbi),
		uintptr(0),
	)
	if status != 0 {
		return fmt.Errorf(`NtQueryInformationProcess failed with NT status 0x%08X`, status)
	}

	commandLine := pbi.PebBaseAddress.ProcessParameters.CommandLine
}

So far, so good. CommandLine is a NTUnicodeString structure. It has the fields Buffer, Length and MaximumLength and the convenience functions Slice and String to convert the Buffer into something useable by Go.

However, there is a caveat: As documented in Microsoft Learn the fields Length and MaximumLength count the lengths in units of bytes, not wchar_t (uint16)!

To get the character count one needs to halve these lengths.

The function *NTUnicodeString.Slice() in golang.org\x\[email protected]\windows has this code:

// Slice returns a uint16 slice that aliases the data in the NTUnicodeString.
func (s *NTUnicodeString) Slice() []uint16 {
	slice := unsafe.Slice(s.Buffer, s.MaximumLength)
	return slice[:s.Length]
}

I.e., it treats the lengths in units of uint16!

What did you see happen?

When I convert the command line buffer to a slice with *NTUnicodeString.Slice() it has a length that is double the size of the Buffer and it contains the content of the *NTUnicodeString.Buffer twice.

Changing the elements of this slice in the second half does not have any influence on the real PEB command line.
Changing the elements of the first half of this slice changes the real PEB command line.

What did you expect to see?

The slice returned by *NTUnicodeString.Slice() (and correspondingly the string returned by *NTUnicodeString.String()) should have a length that is half the value of *NTUnicodeString.Length and a capacity that is half the value of *NTUnicodeString.MaximumLength. It should also contain only one copy of the command line and changes to it should immediately be visible in the PEB of the process.

@gabyhelp
Copy link

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Apr 21, 2025
@randall77
Copy link
Contributor

I agree it looks like (*NTUnicodeString).Slice is wrong. It is using a byte length to slice into a []uint16.
NewNTUnicodeString looks like it does the right thing. It stores 2*(# of uint16s) as the length.

@randall77
Copy link
Contributor

randall77 commented Apr 22, 2025

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/667235 mentions this issue: windows: fix slicing of NTUnicodeString values

@seankhliao seankhliao changed the title golang.org/x/sys/windows (v0.32.0): NTUnicodeString of PROCESS_BASIC_INFORMATION.PebBaseAddress.ProcessParameters.CommandLine is incorrectly converted to slice x/sys/windows: NTUnicodeString of PROCESS_BASIC_INFORMATION.PebBaseAddress.ProcessParameters.CommandLine is incorrectly converted to slice Apr 22, 2025
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 22, 2025
@gopherbot gopherbot added this to the Unreleased milestone Apr 22, 2025
@JunyangShao
Copy link
Contributor

@alexbrainman

@JunyangShao JunyangShao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 22, 2025
@xformerfhs
Copy link
Author

May I say that I very much appreciate the way this issue has been handled. An incredibly quick response. A very quick and factual assessment and an equally quick correction. I am impressed. This is something quite extraordinary these times.

Thank you very much. Keep up the wonderful work. I really love Go and I appreciate the people behind the language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants