diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ce0fad..9377542 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Deprecated +## [0.10.4] + +### Fixed + +- Fixed a crash when splitting command-line arguments under Windows. #124 + ## [0.10.3] ### Fixed diff --git a/sys/windows/syscall_windows.go b/sys/windows/syscall_windows.go index 7da8a07..0c11fda 100644 --- a/sys/windows/syscall_windows.go +++ b/sys/windows/syscall_windows.go @@ -503,9 +503,17 @@ func GetUserProcessParams(handle syscall.Handle, pbi ProcessBasicInformation) (p return params, nil } +// ReadProcessUnicodeString returns a zero-terminated UTF-16 string from another +// process's memory. func ReadProcessUnicodeString(handle syscall.Handle, s *UnicodeString) ([]byte, error) { - buf := make([]byte, s.Size) - nRead, err := ReadProcessMemory(handle, s.Buffer, buf) + // Allocate an extra UTF-16 null character at the end in case the read string + // is not terminated. + extra := 2 + if s.Size&1 != 0 { + extra = 3 // If size is odd, need 3 nulls to terminate. + } + buf := make([]byte, int(s.Size)+extra) + nRead, err := ReadProcessMemory(handle, s.Buffer, buf[:s.Size]) if err != nil { return nil, err } @@ -515,12 +523,26 @@ func ReadProcessUnicodeString(handle syscall.Handle, s *UnicodeString) ([]byte, return buf, nil } -// Use Windows' CommandLineToArgv API to split an UTF-16 command line string -// into a list of parameters. +// ByteSliceToStringSlice uses CommandLineToArgv API to split an UTF-16 command +// line string into a list of parameters. func ByteSliceToStringSlice(utf16 []byte) ([]string, error) { - if len(utf16) == 0 { + n := len(utf16) + // Discard odd byte + if n&1 != 0 { + n-- + utf16 = utf16[:n] + } + if n == 0 { return nil, nil } + terminated := false + for i := 0; i < n && !terminated; i += 2 { + terminated = utf16[i] == 0 && utf16[i+1] == 0 + } + if !terminated { + // Append a null uint16 at the end if terminator is missing + utf16 = append(utf16, 0, 0) + } var numArgs int32 argsWide, err := syscall.CommandLineToArgv((*uint16)(unsafe.Pointer(&utf16[0])), &numArgs) if err != nil { diff --git a/sys/windows/syscall_windows_test.go b/sys/windows/syscall_windows_test.go index 6178e4a..61eff28 100644 --- a/sys/windows/syscall_windows_test.go +++ b/sys/windows/syscall_windows_test.go @@ -224,6 +224,7 @@ func TestReadProcessUnicodeString(t *testing.T) { if err != nil { t.Fatal(err) } + defer syscall.CloseHandle(h) info, err := NtQueryProcessBasicInformation(h) if err != nil { t.Fatal(err) @@ -236,10 +237,36 @@ func TestReadProcessUnicodeString(t *testing.T) { if err != nil { t.Fatal(err) } - defer syscall.CloseHandle(h) assert.NoError(t, err) assert.NotEmpty(t, read) } + +const currentProcessHandle = syscall.Handle(^uintptr(0)) + +func TestReadProcessUnicodeStringTerminator(t *testing.T) { + data := []byte{'H', 0, 'E', 0, 'L', 0, 'L', 0, 'O', 0, 0, 0} + for n := len(data); n >= 0; n-- { + us := UnicodeString{ + Buffer: uintptr(unsafe.Pointer(&data[0])), + Size: uint16(n), + } + read, err := ReadProcessUnicodeString(currentProcessHandle, &us) + if err != nil { + t.Fatal(err) + } + nRead := len(read) + // Strings must match + assert.True(t, nRead >= n) + assert.Equal(t, data[:n], read[:n]) + // result is an array of uint16, can't have odd length. + assert.True(t, nRead&1 == 0) + // Must include a zero terminator at the end. + assert.True(t, nRead >= 2) + assert.Zero(t, read[nRead-1]) + assert.Zero(t, read[nRead-2]) + } +} + func TestReadProcessUnicodeStringInvalidHandle(t *testing.T) { var handle syscall.Handle var cmd = UnicodeString{Size: 5, MaximumLength: 400, Buffer: 400} @@ -264,6 +291,33 @@ func TestByteSliceToStringSliceEmptyBytes(t *testing.T) { assert.Empty(t, cmd) } +func mkUtf16bytes(s string) []byte { + n := len(s) + b := make([]byte, n*2) + for idx, val := range s { + *(*uint16)(unsafe.Pointer(&b[idx*2])) = uint16(val) + } + return b +} + +func TestByteSliceToStringSliceNotTerminated(t *testing.T) { + b := mkUtf16bytes("Hello World") + cmd, err := ByteSliceToStringSlice(b) + assert.NoError(t, err) + assert.Len(t, cmd, 2) + assert.Equal(t, "Hello", cmd[0]) + assert.Equal(t, "World", cmd[1]) +} + +func TestByteSliceToStringSliceNotOddSize(t *testing.T) { + b := mkUtf16bytes("BAD")[:5] + cmd, err := ByteSliceToStringSlice(b) + assert.NoError(t, err) + assert.Len(t, cmd, 1) + // Odd character is dropped + assert.Equal(t, "BA", cmd[0]) +} + func TestReadProcessMemory(t *testing.T) { h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION|PROCESS_VM_READ, false, uint32(syscall.Getpid())) if err != nil {