Skip to content

Commit 1b2bd6b

Browse files
tekkamanendlessgopherbot
authored andcommitted
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>
1 parent 1c3b72f commit 1b2bd6b

File tree

8 files changed

+132
-30
lines changed

8 files changed

+132
-30
lines changed

windows/registry/key.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,12 @@ loopItems:
164164
func CreateKey(k Key, path string, access uint32) (newk Key, openedExisting bool, err error) {
165165
var h syscall.Handle
166166
var d uint32
167-
err = regCreateKeyEx(syscall.Handle(k), syscall.StringToUTF16Ptr(path),
167+
var pathPointer *uint16
168+
pathPointer, err = syscall.UTF16PtrFromString(path)
169+
if err != nil {
170+
return 0, false, err
171+
}
172+
err = regCreateKeyEx(syscall.Handle(k), pathPointer,
168173
0, nil, _REG_OPTION_NON_VOLATILE, access, nil, &h, &d)
169174
if err != nil {
170175
return 0, false, err
@@ -174,7 +179,11 @@ func CreateKey(k Key, path string, access uint32) (newk Key, openedExisting bool
174179

175180
// DeleteKey deletes the subkey path of key k and its values.
176181
func DeleteKey(k Key, path string) error {
177-
return regDeleteKey(syscall.Handle(k), syscall.StringToUTF16Ptr(path))
182+
pathPointer, err := syscall.UTF16PtrFromString(path)
183+
if err != nil {
184+
return err
185+
}
186+
return regDeleteKey(syscall.Handle(k), pathPointer)
178187
}
179188

180189
// A KeyInfo describes the statistics of a key. It is returned by Stat.

windows/registry/value.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,11 @@ func (k Key) SetBinaryValue(name string, value []byte) error {
340340

341341
// DeleteValue removes a named value from the key k.
342342
func (k Key) DeleteValue(name string) error {
343-
return regDeleteValue(syscall.Handle(k), syscall.StringToUTF16Ptr(name))
343+
namePointer, err := syscall.UTF16PtrFromString(name)
344+
if err != nil {
345+
return err
346+
}
347+
return regDeleteValue(syscall.Handle(k), namePointer)
344348
}
345349

346350
// ReadValueNames returns the value names of key k.

windows/svc/eventlog/log.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,19 @@ func OpenRemote(host, source string) (*Log, error) {
2929
if source == "" {
3030
return nil, errors.New("Specify event log source")
3131
}
32-
var s *uint16
32+
var hostPointer *uint16
3333
if host != "" {
34-
s = syscall.StringToUTF16Ptr(host)
34+
var err error
35+
hostPointer, err = syscall.UTF16PtrFromString(host)
36+
if err != nil {
37+
return nil, err
38+
}
3539
}
36-
h, err := windows.RegisterEventSource(s, syscall.StringToUTF16Ptr(source))
40+
sourcePointer, err := syscall.UTF16PtrFromString(source)
41+
if err != nil {
42+
return nil, err
43+
}
44+
h, err := windows.RegisterEventSource(hostPointer, sourcePointer)
3745
if err != nil {
3846
return nil, err
3947
}
@@ -46,7 +54,11 @@ func (l *Log) Close() error {
4654
}
4755

4856
func (l *Log) report(etype uint16, eid uint32, msg string) error {
49-
ss := []*uint16{syscall.StringToUTF16Ptr(msg)}
57+
msgPointer, err := syscall.UTF16PtrFromString(msg)
58+
if err != nil {
59+
return err
60+
}
61+
ss := []*uint16{msgPointer}
5062
return windows.ReportEvent(l.Handle, etype, 0, eid, 0, 1, 0, &ss[0], nil)
5163
}
5264

windows/svc/mgr/config.go

+29-5
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,11 @@ func (s *Service) Config() (Config, error) {
121121
}
122122

123123
func updateDescription(handle windows.Handle, desc string) error {
124-
d := windows.SERVICE_DESCRIPTION{Description: toPtr(desc)}
124+
descPointer, err := toPtr(desc)
125+
if err != nil {
126+
return err
127+
}
128+
d := windows.SERVICE_DESCRIPTION{Description: descPointer}
125129
return windows.ChangeServiceConfig2(handle,
126130
windows.SERVICE_CONFIG_DESCRIPTION, (*byte)(unsafe.Pointer(&d)))
127131
}
@@ -141,10 +145,30 @@ func updateStartUp(handle windows.Handle, isDelayed bool) error {
141145

142146
// UpdateConfig updates service s configuration parameters.
143147
func (s *Service) UpdateConfig(c Config) error {
144-
err := windows.ChangeServiceConfig(s.Handle, c.ServiceType, c.StartType,
145-
c.ErrorControl, toPtr(c.BinaryPathName), toPtr(c.LoadOrderGroup),
146-
nil, toStringBlock(c.Dependencies), toPtr(c.ServiceStartName),
147-
toPtr(c.Password), toPtr(c.DisplayName))
148+
binaryPathNamePointer, err := toPtr(c.BinaryPathName)
149+
if err != nil {
150+
return err
151+
}
152+
loadOrderGroupPointer, err := toPtr(c.LoadOrderGroup)
153+
if err != nil {
154+
return err
155+
}
156+
serviceStartNamePointer, err := toPtr(c.ServiceStartName)
157+
if err != nil {
158+
return err
159+
}
160+
passwordPointer, err := toPtr(c.Password)
161+
if err != nil {
162+
return err
163+
}
164+
displayNamePointer, err := toPtr(c.DisplayName)
165+
if err != nil {
166+
return err
167+
}
168+
err = windows.ChangeServiceConfig(s.Handle, c.ServiceType, c.StartType,
169+
c.ErrorControl, binaryPathNamePointer, loadOrderGroupPointer,
170+
nil, toStringBlock(c.Dependencies), serviceStartNamePointer,
171+
passwordPointer, displayNamePointer)
148172
if err != nil {
149173
return err
150174
}

windows/svc/mgr/mgr.go

+41-8
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ func Connect() (*Mgr, error) {
3434
func ConnectRemote(host string) (*Mgr, error) {
3535
var s *uint16
3636
if host != "" {
37-
s = syscall.StringToUTF16Ptr(host)
37+
var err error
38+
s, err = syscall.UTF16PtrFromString(host)
39+
if err != nil {
40+
return nil, err
41+
}
3842
}
3943
h, err := windows.OpenSCManager(s, nil, windows.SC_MANAGER_ALL_ACCESS)
4044
if err != nil {
@@ -78,11 +82,11 @@ func (m *Mgr) LockStatus() (*LockStatus, error) {
7882
}
7983
}
8084

81-
func toPtr(s string) *uint16 {
85+
func toPtr(s string) (*uint16, error) {
8286
if len(s) == 0 {
83-
return nil
87+
return nil, nil
8488
}
85-
return syscall.StringToUTF16Ptr(s)
89+
return syscall.UTF16PtrFromString(s)
8690
}
8791

8892
// toStringBlock terminates strings in ss with 0, and then
@@ -122,10 +126,34 @@ func (m *Mgr) CreateService(name, exepath string, c Config, args ...string) (*Se
122126
for _, v := range args {
123127
s += " " + syscall.EscapeArg(v)
124128
}
125-
h, err := windows.CreateService(m.Handle, toPtr(name), toPtr(c.DisplayName),
129+
namePointer, err := toPtr(name)
130+
if err != nil {
131+
return nil, err
132+
}
133+
displayNamePointer, err := toPtr(c.DisplayName)
134+
if err != nil {
135+
return nil, err
136+
}
137+
sPointer, err := toPtr(s)
138+
if err != nil {
139+
return nil, err
140+
}
141+
loadOrderGroupPointer, err := toPtr(c.LoadOrderGroup)
142+
if err != nil {
143+
return nil, err
144+
}
145+
serviceStartNamePointer, err := toPtr(c.ServiceStartName)
146+
if err != nil {
147+
return nil, err
148+
}
149+
passwordPointer, err := toPtr(c.Password)
150+
if err != nil {
151+
return nil, err
152+
}
153+
h, err := windows.CreateService(m.Handle, namePointer, displayNamePointer,
126154
windows.SERVICE_ALL_ACCESS, c.ServiceType,
127-
c.StartType, c.ErrorControl, toPtr(s), toPtr(c.LoadOrderGroup),
128-
nil, toStringBlock(c.Dependencies), toPtr(c.ServiceStartName), toPtr(c.Password))
155+
c.StartType, c.ErrorControl, sPointer, loadOrderGroupPointer,
156+
nil, toStringBlock(c.Dependencies), serviceStartNamePointer, passwordPointer)
129157
if err != nil {
130158
return nil, err
131159
}
@@ -159,7 +187,12 @@ func (m *Mgr) CreateService(name, exepath string, c Config, args ...string) (*Se
159187
// OpenService retrieves access to service name, so it can
160188
// be interrogated and controlled.
161189
func (m *Mgr) OpenService(name string) (*Service, error) {
162-
h, err := windows.OpenService(m.Handle, syscall.StringToUTF16Ptr(name), windows.SERVICE_ALL_ACCESS)
190+
namePointer, err := syscall.UTF16PtrFromString(name)
191+
if err != nil {
192+
return nil, err
193+
}
194+
195+
h, err := windows.OpenService(m.Handle, namePointer, windows.SERVICE_ALL_ACCESS)
163196
if err != nil {
164197
return nil, err
165198
}

windows/svc/mgr/recovery.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,13 @@ func (s *Service) ResetPeriod() (uint32, error) {
9999
// SetRebootMessage sets service s reboot message.
100100
// If msg is "", the reboot message is deleted and no message is broadcast.
101101
func (s *Service) SetRebootMessage(msg string) error {
102+
msgPointer, err := syscall.UTF16PtrFromString(msg)
103+
if err != nil {
104+
return err
105+
}
106+
102107
rActions := windows.SERVICE_FAILURE_ACTIONS{
103-
RebootMsg: syscall.StringToUTF16Ptr(msg),
108+
RebootMsg: msgPointer,
104109
}
105110
return windows.ChangeServiceConfig2(s.Handle, windows.SERVICE_CONFIG_FAILURE_ACTIONS, (*byte)(unsafe.Pointer(&rActions)))
106111
}
@@ -118,8 +123,13 @@ func (s *Service) RebootMessage() (string, error) {
118123
// SetRecoveryCommand sets the command line of the process to execute in response to the RunCommand service controller action.
119124
// If cmd is "", the command is deleted and no program is run when the service fails.
120125
func (s *Service) SetRecoveryCommand(cmd string) error {
126+
cmdPointer, err := syscall.UTF16PtrFromString(cmd)
127+
if err != nil {
128+
return err
129+
}
130+
121131
rActions := windows.SERVICE_FAILURE_ACTIONS{
122-
Command: syscall.StringToUTF16Ptr(cmd),
132+
Command: cmdPointer,
123133
}
124134
return windows.ChangeServiceConfig2(s.Handle, windows.SERVICE_CONFIG_FAILURE_ACTIONS, (*byte)(unsafe.Pointer(&rActions)))
125135
}

windows/svc/mgr/service.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ func (s *Service) Start(args ...string) error {
3737
if len(args) > 0 {
3838
vs := make([]*uint16, len(args))
3939
for i := range vs {
40-
vs[i] = syscall.StringToUTF16Ptr(args[i])
40+
argPointer, err := syscall.UTF16PtrFromString(args[i])
41+
if err != nil {
42+
return err
43+
}
44+
vs[i] = argPointer
4145
}
4246
p = &vs[0]
4347
}

windows/svc/service.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ type ctlEvent struct {
132132

133133
// service provides access to windows service api.
134134
type service struct {
135-
name string
136-
h windows.Handle
137-
c chan ctlEvent
138-
handler Handler
135+
namePointer *uint16
136+
h windows.Handle
137+
c chan ctlEvent
138+
handler Handler
139139
}
140140

141141
type exitCode struct {
@@ -209,7 +209,7 @@ var theService service // This is, unfortunately, a global, which means only one
209209
// serviceMain is the entry point called by the service manager, registered earlier by
210210
// the call to StartServiceCtrlDispatcher.
211211
func serviceMain(argc uint32, argv **uint16) uintptr {
212-
handle, err := windows.RegisterServiceCtrlHandlerEx(windows.StringToUTF16Ptr(theService.name), ctlHandlerCallback, 0)
212+
handle, err := windows.RegisterServiceCtrlHandlerEx(theService.namePointer, ctlHandlerCallback, 0)
213213
if sysErr, ok := err.(windows.Errno); ok {
214214
return uintptr(sysErr)
215215
} else if err != nil {
@@ -280,15 +280,21 @@ loop:
280280

281281
// Run executes service name by calling appropriate handler function.
282282
func Run(name string, handler Handler) error {
283+
// Check to make sure that the service name is valid.
284+
namePointer, err := windows.UTF16PtrFromString(name)
285+
if err != nil {
286+
return err
287+
}
288+
283289
initCallbacks.Do(func() {
284290
ctlHandlerCallback = windows.NewCallback(ctlHandler)
285291
serviceMainCallback = windows.NewCallback(serviceMain)
286292
})
287-
theService.name = name
293+
theService.namePointer = namePointer
288294
theService.handler = handler
289295
theService.c = make(chan ctlEvent)
290296
t := []windows.SERVICE_TABLE_ENTRY{
291-
{ServiceName: windows.StringToUTF16Ptr(theService.name), ServiceProc: serviceMainCallback},
297+
{ServiceName: namePointer, ServiceProc: serviceMainCallback},
292298
{ServiceName: nil, ServiceProc: 0},
293299
}
294300
return windows.StartServiceCtrlDispatcher(&t[0])

0 commit comments

Comments
 (0)