From 021444007590da4c1f6e504904e2871a1012c0bf Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 13 May 2021 13:19:14 +0200 Subject: [PATCH] syscall: do not pass console handles to PROC_THREAD_ATTRIBUTE_HANDLE_LIST on Windows 7 On Windows 7 (and below), console handles are not real kernel handles but are rather userspace objects, with information passed via special bits in the handle itself. That means they can't be passed in PROC_THREAD_ATTRIBUTE_HANDLE_LIST, even though they can be inherited. So, we filter the list passed to PROC_THREAD_ATTRIBUTE_HANDLE_LIST to not have any console handles on Windows 7. At the same time, it turns out that the presence of a NULL handle in the list is enough to render PROC_THREAD_ATTRIBUTE_HANDLE_LIST completely useless, so filter these out too. Console handles also can't be duplicated into parent processes, as inhertance always happens from the present process, so duplicate always into the present process even when a parent process is specified. Fixes #45914. Change-Id: I70b4ff4874dbf0507d9ec9278f63b9b4dd4f1999 Reviewed-on: https://go-review.googlesource.com/c/go/+/319310 Trust: Jason A. Donenfeld Trust: Alex Brainman Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Alex Brainman --- src/syscall/exec_windows.go | 54 ++++++++++++++++++++++++++++----- src/syscall/syscall_windows.go | 1 + src/syscall/zsyscall_windows.go | 7 +++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go index 253e9e8c1f9..18d15028c3d 100644 --- a/src/syscall/exec_windows.go +++ b/src/syscall/exec_windows.go @@ -313,6 +313,17 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle } } + var maj, min, build uint32 + rtlGetNtVersionNumbers(&maj, &min, &build) + isWin7 := maj < 6 || (maj == 6 && min <= 1) + // NT kernel handles are divisible by 4, with the bottom 3 bits left as + // a tag. The fully set tag correlates with the types of handles we're + // concerned about here. Except, the kernel will interpret some + // special handle values, like -1, -2, and so forth, so kernelbase.dll + // checks to see that those bottom three bits are checked, but that top + // bit is not checked. + isLegacyWin7ConsoleHandle := func(handle Handle) bool { return isWin7 && handle&0x10000003 == 3 } + p, _ := GetCurrentProcess() parentProcess := p if sys.ParentProcess != 0 { @@ -321,7 +332,15 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle fd := make([]Handle, len(attr.Files)) for i := range attr.Files { if attr.Files[i] > 0 { - err := DuplicateHandle(p, Handle(attr.Files[i]), parentProcess, &fd[i], 0, true, DUPLICATE_SAME_ACCESS) + destinationProcessHandle := parentProcess + + // On Windows 7, console handles aren't real handles, and can only be duplicated + // into the current process, not a parent one, which amounts to the same thing. + if parentProcess != p && isLegacyWin7ConsoleHandle(Handle(attr.Files[i])) { + destinationProcessHandle = p + } + + err := DuplicateHandle(p, Handle(attr.Files[i]), destinationProcessHandle, &fd[i], 0, true, DUPLICATE_SAME_ACCESS) if err != nil { return 0, 0, err } @@ -351,19 +370,40 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle si.StdErr = fd[2] fd = append(fd, sys.AdditionalInheritedHandles...) + + // On Windows 7, console handles aren't real handles, so don't pass them + // through to PROC_THREAD_ATTRIBUTE_HANDLE_LIST. + for i := range fd { + if isLegacyWin7ConsoleHandle(fd[i]) { + fd[i] = 0 + } + } + + // The presence of a NULL handle in the list is enough to cause PROC_THREAD_ATTRIBUTE_HANDLE_LIST + // to treat the entire list as empty, so remove NULL handles. + j := 0 + for i := range fd { + if fd[i] != 0 { + fd[j] = fd[i] + j++ + } + } + fd = fd[:j] + // Do not accidentally inherit more than these handles. - err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil) - if err != nil { - return 0, 0, err + if len(fd) > 0 { + err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil) + if err != nil { + return 0, 0, err + } } pi := new(ProcessInformation) - flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT if sys.Token != 0 { - err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, len(fd) > 0 && !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } else { - err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, len(fd) > 0 && !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } if err != nil { return 0, 0, err diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index fc734effbbe..660179ae9e8 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -198,6 +198,7 @@ func NewCallbackCDecl(fn interface{}) uintptr { //sys FreeLibrary(handle Handle) (err error) //sys GetProcAddress(module Handle, procname string) (proc uintptr, err error) //sys GetVersion() (ver uint32, err error) +//sys rtlGetNtVersionNumbers(majorVersion *uint32, minorVersion *uint32, buildNumber *uint32) = ntdll.RtlGetNtVersionNumbers //sys formatMessage(flags uint32, msgsrc uintptr, msgid uint32, langid uint32, buf []uint16, args *byte) (n uint32, err error) = FormatMessageW //sys ExitProcess(exitcode uint32) //sys CreateFile(name *uint16, access uint32, mode uint32, sa *SecurityAttributes, createmode uint32, attrs uint32, templatefile int32) (handle Handle, err error) [failretval==InvalidHandle] = CreateFileW diff --git a/src/syscall/zsyscall_windows.go b/src/syscall/zsyscall_windows.go index 10d0f54e8c7..b9e429693d7 100644 --- a/src/syscall/zsyscall_windows.go +++ b/src/syscall/zsyscall_windows.go @@ -41,6 +41,7 @@ var ( moddnsapi = NewLazyDLL(sysdll.Add("dnsapi.dll")) modiphlpapi = NewLazyDLL(sysdll.Add("iphlpapi.dll")) modkernel32 = NewLazyDLL(sysdll.Add("kernel32.dll")) + modntdll = NewLazyDLL(sysdll.Add("ntdll.dll")) modmswsock = NewLazyDLL(sysdll.Add("mswsock.dll")) modnetapi32 = NewLazyDLL(sysdll.Add("netapi32.dll")) modsecur32 = NewLazyDLL(sysdll.Add("secur32.dll")) @@ -167,6 +168,7 @@ var ( procNetApiBufferFree = modnetapi32.NewProc("NetApiBufferFree") procNetGetJoinInformation = modnetapi32.NewProc("NetGetJoinInformation") procNetUserGetInfo = modnetapi32.NewProc("NetUserGetInfo") + procRtlGetNtVersionNumbers = modntdll.NewProc("RtlGetNtVersionNumbers") procGetUserNameExW = modsecur32.NewProc("GetUserNameExW") procTranslateNameW = modsecur32.NewProc("TranslateNameW") procCommandLineToArgvW = modshell32.NewProc("CommandLineToArgvW") @@ -1213,6 +1215,11 @@ func NetUserGetInfo(serverName *uint16, userName *uint16, level uint32, buf **by return } +func rtlGetNtVersionNumbers(majorVersion *uint32, minorVersion *uint32, buildNumber *uint32) { + Syscall(procRtlGetNtVersionNumbers.Addr(), 3, uintptr(unsafe.Pointer(majorVersion)), uintptr(unsafe.Pointer(minorVersion)), uintptr(unsafe.Pointer(buildNumber))) + return +} + func GetUserNameEx(nameFormat uint32, nameBuffre *uint16, nSize *uint32) (err error) { r1, _, e1 := Syscall(procGetUserNameExW.Addr(), 3, uintptr(nameFormat), uintptr(unsafe.Pointer(nameBuffre)), uintptr(unsafe.Pointer(nSize))) if r1&0xff == 0 {