windows: replace all StringToUTF16 calls with UTF16FromString `StringToUTF16` is deprecated and will panic if given an "invalid" string (in particular, one that has a null byte in it). The replacement function is `UTF16FromString`, and it returns an error if there was a problem. This change replaces all uses of `StringToUTF16` with `UTF16FromString`. The `service` struct now no longer stores a `string` name but rather a `*uint16` pointer to the name. It should not be possible to panic due to UTF16 string conversion at this point. Fixes golang/go#73006 Change-Id: Idce9cdbb4651fef8481f0cad19b5df0314fd4277 Reviewed-on: https://go-review.googlesource.com/c/sys/+/659936 Reviewed-by: Carlos Amedee <carlos@golang.org> Auto-Submit: Carlos Amedee <carlos@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/windows/registry/key.go b/windows/registry/key.go index fd86324..39aeeb6 100644 --- a/windows/registry/key.go +++ b/windows/registry/key.go
@@ -164,7 +164,12 @@ func CreateKey(k Key, path string, access uint32) (newk Key, openedExisting bool, err error) { var h syscall.Handle var d uint32 - err = regCreateKeyEx(syscall.Handle(k), syscall.StringToUTF16Ptr(path), + var pathPointer *uint16 + pathPointer, err = syscall.UTF16PtrFromString(path) + if err != nil { + return 0, false, err + } + err = regCreateKeyEx(syscall.Handle(k), pathPointer, 0, nil, _REG_OPTION_NON_VOLATILE, access, nil, &h, &d) if err != nil { return 0, false, err @@ -174,7 +179,11 @@ // DeleteKey deletes the subkey path of key k and its values. func DeleteKey(k Key, path string) error { - return regDeleteKey(syscall.Handle(k), syscall.StringToUTF16Ptr(path)) + pathPointer, err := syscall.UTF16PtrFromString(path) + if err != nil { + return err + } + return regDeleteKey(syscall.Handle(k), pathPointer) } // A KeyInfo describes the statistics of a key. It is returned by Stat.
diff --git a/windows/registry/value.go b/windows/registry/value.go index 74db26b..a1bcbb2 100644 --- a/windows/registry/value.go +++ b/windows/registry/value.go
@@ -340,7 +340,11 @@ // DeleteValue removes a named value from the key k. func (k Key) DeleteValue(name string) error { - return regDeleteValue(syscall.Handle(k), syscall.StringToUTF16Ptr(name)) + namePointer, err := syscall.UTF16PtrFromString(name) + if err != nil { + return err + } + return regDeleteValue(syscall.Handle(k), namePointer) } // ReadValueNames returns the value names of key k.
diff --git a/windows/svc/eventlog/log.go b/windows/svc/eventlog/log.go index f279444..ad40c2f 100644 --- a/windows/svc/eventlog/log.go +++ b/windows/svc/eventlog/log.go
@@ -29,11 +29,19 @@ if source == "" { return nil, errors.New("Specify event log source") } - var s *uint16 + var hostPointer *uint16 if host != "" { - s = syscall.StringToUTF16Ptr(host) + var err error + hostPointer, err = syscall.UTF16PtrFromString(host) + if err != nil { + return nil, err + } } - h, err := windows.RegisterEventSource(s, syscall.StringToUTF16Ptr(source)) + sourcePointer, err := syscall.UTF16PtrFromString(source) + if err != nil { + return nil, err + } + h, err := windows.RegisterEventSource(hostPointer, sourcePointer) if err != nil { return nil, err } @@ -46,7 +54,11 @@ } func (l *Log) report(etype uint16, eid uint32, msg string) error { - ss := []*uint16{syscall.StringToUTF16Ptr(msg)} + msgPointer, err := syscall.UTF16PtrFromString(msg) + if err != nil { + return err + } + ss := []*uint16{msgPointer} return windows.ReportEvent(l.Handle, etype, 0, eid, 0, 1, 0, &ss[0], nil) }
diff --git a/windows/svc/mgr/config.go b/windows/svc/mgr/config.go index 3c7ba08..ec01f96 100644 --- a/windows/svc/mgr/config.go +++ b/windows/svc/mgr/config.go
@@ -121,7 +121,11 @@ } func updateDescription(handle windows.Handle, desc string) error { - d := windows.SERVICE_DESCRIPTION{Description: toPtr(desc)} + descPointer, err := toPtr(desc) + if err != nil { + return err + } + d := windows.SERVICE_DESCRIPTION{Description: descPointer} return windows.ChangeServiceConfig2(handle, windows.SERVICE_CONFIG_DESCRIPTION, (*byte)(unsafe.Pointer(&d))) } @@ -141,10 +145,30 @@ // UpdateConfig updates service s configuration parameters. func (s *Service) UpdateConfig(c Config) error { - err := windows.ChangeServiceConfig(s.Handle, c.ServiceType, c.StartType, - c.ErrorControl, toPtr(c.BinaryPathName), toPtr(c.LoadOrderGroup), - nil, toStringBlock(c.Dependencies), toPtr(c.ServiceStartName), - toPtr(c.Password), toPtr(c.DisplayName)) + binaryPathNamePointer, err := toPtr(c.BinaryPathName) + if err != nil { + return err + } + loadOrderGroupPointer, err := toPtr(c.LoadOrderGroup) + if err != nil { + return err + } + serviceStartNamePointer, err := toPtr(c.ServiceStartName) + if err != nil { + return err + } + passwordPointer, err := toPtr(c.Password) + if err != nil { + return err + } + displayNamePointer, err := toPtr(c.DisplayName) + if err != nil { + return err + } + err = windows.ChangeServiceConfig(s.Handle, c.ServiceType, c.StartType, + c.ErrorControl, binaryPathNamePointer, loadOrderGroupPointer, + nil, toStringBlock(c.Dependencies), serviceStartNamePointer, + passwordPointer, displayNamePointer) if err != nil { return err }
diff --git a/windows/svc/mgr/mgr.go b/windows/svc/mgr/mgr.go index dbfd729..7475594 100644 --- a/windows/svc/mgr/mgr.go +++ b/windows/svc/mgr/mgr.go
@@ -34,7 +34,11 @@ func ConnectRemote(host string) (*Mgr, error) { var s *uint16 if host != "" { - s = syscall.StringToUTF16Ptr(host) + var err error + s, err = syscall.UTF16PtrFromString(host) + if err != nil { + return nil, err + } } h, err := windows.OpenSCManager(s, nil, windows.SC_MANAGER_ALL_ACCESS) if err != nil { @@ -78,11 +82,11 @@ } } -func toPtr(s string) *uint16 { +func toPtr(s string) (*uint16, error) { if len(s) == 0 { - return nil + return nil, nil } - return syscall.StringToUTF16Ptr(s) + return syscall.UTF16PtrFromString(s) } // toStringBlock terminates strings in ss with 0, and then @@ -122,10 +126,34 @@ for _, v := range args { s += " " + syscall.EscapeArg(v) } - h, err := windows.CreateService(m.Handle, toPtr(name), toPtr(c.DisplayName), + namePointer, err := toPtr(name) + if err != nil { + return nil, err + } + displayNamePointer, err := toPtr(c.DisplayName) + if err != nil { + return nil, err + } + sPointer, err := toPtr(s) + if err != nil { + return nil, err + } + loadOrderGroupPointer, err := toPtr(c.LoadOrderGroup) + if err != nil { + return nil, err + } + serviceStartNamePointer, err := toPtr(c.ServiceStartName) + if err != nil { + return nil, err + } + passwordPointer, err := toPtr(c.Password) + if err != nil { + return nil, err + } + h, err := windows.CreateService(m.Handle, namePointer, displayNamePointer, windows.SERVICE_ALL_ACCESS, c.ServiceType, - c.StartType, c.ErrorControl, toPtr(s), toPtr(c.LoadOrderGroup), - nil, toStringBlock(c.Dependencies), toPtr(c.ServiceStartName), toPtr(c.Password)) + c.StartType, c.ErrorControl, sPointer, loadOrderGroupPointer, + nil, toStringBlock(c.Dependencies), serviceStartNamePointer, passwordPointer) if err != nil { return nil, err } @@ -159,7 +187,12 @@ // OpenService retrieves access to service name, so it can // be interrogated and controlled. func (m *Mgr) OpenService(name string) (*Service, error) { - h, err := windows.OpenService(m.Handle, syscall.StringToUTF16Ptr(name), windows.SERVICE_ALL_ACCESS) + namePointer, err := syscall.UTF16PtrFromString(name) + if err != nil { + return nil, err + } + + h, err := windows.OpenService(m.Handle, namePointer, windows.SERVICE_ALL_ACCESS) if err != nil { return nil, err }
diff --git a/windows/svc/mgr/recovery.go b/windows/svc/mgr/recovery.go index ef2a687..1a07374 100644 --- a/windows/svc/mgr/recovery.go +++ b/windows/svc/mgr/recovery.go
@@ -99,8 +99,13 @@ // SetRebootMessage sets service s reboot message. // If msg is "", the reboot message is deleted and no message is broadcast. func (s *Service) SetRebootMessage(msg string) error { + msgPointer, err := syscall.UTF16PtrFromString(msg) + if err != nil { + return err + } + rActions := windows.SERVICE_FAILURE_ACTIONS{ - RebootMsg: syscall.StringToUTF16Ptr(msg), + RebootMsg: msgPointer, } return windows.ChangeServiceConfig2(s.Handle, windows.SERVICE_CONFIG_FAILURE_ACTIONS, (*byte)(unsafe.Pointer(&rActions))) } @@ -118,8 +123,13 @@ // SetRecoveryCommand sets the command line of the process to execute in response to the RunCommand service controller action. // If cmd is "", the command is deleted and no program is run when the service fails. func (s *Service) SetRecoveryCommand(cmd string) error { + cmdPointer, err := syscall.UTF16PtrFromString(cmd) + if err != nil { + return err + } + rActions := windows.SERVICE_FAILURE_ACTIONS{ - Command: syscall.StringToUTF16Ptr(cmd), + Command: cmdPointer, } return windows.ChangeServiceConfig2(s.Handle, windows.SERVICE_CONFIG_FAILURE_ACTIONS, (*byte)(unsafe.Pointer(&rActions))) }
diff --git a/windows/svc/mgr/service.go b/windows/svc/mgr/service.go index c9740ef..9f59eab 100644 --- a/windows/svc/mgr/service.go +++ b/windows/svc/mgr/service.go
@@ -37,7 +37,11 @@ if len(args) > 0 { vs := make([]*uint16, len(args)) for i := range vs { - vs[i] = syscall.StringToUTF16Ptr(args[i]) + argPointer, err := syscall.UTF16PtrFromString(args[i]) + if err != nil { + return err + } + vs[i] = argPointer } p = &vs[0] }
diff --git a/windows/svc/service.go b/windows/svc/service.go index c4f7492..a9b1c19 100644 --- a/windows/svc/service.go +++ b/windows/svc/service.go
@@ -132,10 +132,10 @@ // service provides access to windows service api. type service struct { - name string - h windows.Handle - c chan ctlEvent - handler Handler + namePointer *uint16 + h windows.Handle + c chan ctlEvent + handler Handler } type exitCode struct { @@ -209,7 +209,7 @@ // serviceMain is the entry point called by the service manager, registered earlier by // the call to StartServiceCtrlDispatcher. func serviceMain(argc uint32, argv **uint16) uintptr { - handle, err := windows.RegisterServiceCtrlHandlerEx(windows.StringToUTF16Ptr(theService.name), ctlHandlerCallback, 0) + handle, err := windows.RegisterServiceCtrlHandlerEx(theService.namePointer, ctlHandlerCallback, 0) if sysErr, ok := err.(windows.Errno); ok { return uintptr(sysErr) } else if err != nil { @@ -280,15 +280,21 @@ // Run executes service name by calling appropriate handler function. func Run(name string, handler Handler) error { + // Check to make sure that the service name is valid. + namePointer, err := windows.UTF16PtrFromString(name) + if err != nil { + return err + } + initCallbacks.Do(func() { ctlHandlerCallback = windows.NewCallback(ctlHandler) serviceMainCallback = windows.NewCallback(serviceMain) }) - theService.name = name + theService.namePointer = namePointer theService.handler = handler theService.c = make(chan ctlEvent) t := []windows.SERVICE_TABLE_ENTRY{ - {ServiceName: windows.StringToUTF16Ptr(theService.name), ServiceProc: serviceMainCallback}, + {ServiceName: namePointer, ServiceProc: serviceMainCallback}, {ServiceName: nil, ServiceProc: 0}, } return windows.StartServiceCtrlDispatcher(&t[0])