From 7fa72aa124480c95ad1cc5f31657aee54a4f8074 Mon Sep 17 00:00:00 2001 From: Nick Maliwacki Date: Thu, 12 Dec 2019 22:38:54 -0800 Subject: [PATCH] fix on all unsafe.Pointer rules (#8) --- native-powershell | 2 +- pkg/powershell/chelpers.go | 65 +++++++++++++++++++------------ pkg/powershell/hostcommand.go | 15 ++----- pkg/powershell/powershell.go | 3 +- pkg/powershell/psh_host.go | 2 +- pkg/powershell/runspace_test.go | 2 +- pkg/powershell/runspacehelpers.go | 13 ++++--- pkg/powershell/zpsh_host.go | 49 +++++++++++++++++++++-- 8 files changed, 101 insertions(+), 50 deletions(-) diff --git a/native-powershell b/native-powershell index 49419e0..6c94264 160000 --- a/native-powershell +++ b/native-powershell @@ -1 +1 @@ -Subproject commit 49419e083381ced660038977496a1ec0a172a371 +Subproject commit 6c942644cecd123b4b8792e3d04eedf108ae5c22 diff --git a/pkg/powershell/chelpers.go b/pkg/powershell/chelpers.go index 806aea4..4bba2cf 100644 --- a/pkg/powershell/chelpers.go +++ b/pkg/powershell/chelpers.go @@ -2,8 +2,6 @@ package powershell import ( "unsafe" - - "golang.org/x/sys/windows" ) func makeUint64FromPtr(v uintptr) uint64 { @@ -13,18 +11,50 @@ func makeUintptrFromUint64(v uint64) uintptr { return *((*uintptr)(unsafe.Pointer(&v))) } -func allocWrapper(size uint64) uintptr { +func allocWrapper(size uint64) (uintptr, error) { return nativePowerShell_DefaultAlloc(size) } func freeWrapper(v uintptr) { nativePowerShell_DefaultFree(v) } -func mallocCopy(input uintptr, size uintptr) uintptr { +func mallocCopyLogStringHolder(input nativePowerShell_LogString_Holder) uintptr { + + size := uint64(unsafe.Sizeof(input)) + + data, err := allocWrapper(size) + if err != nil { + panic("Couldn't allocate memory") + } + + _ = memcpyLogStringHolder(data, input) + + return data +} + +func mallocCopyGenericPowerShellObject(input *nativePowerShell_GenericPowerShellObject, inputCount uint64) uintptr { + + size := inputCount * uint64(unsafe.Sizeof(*input)) + + data, err := allocWrapper(size) + if err != nil { + panic("Couldn't allocate memory") + } - u64Size := makeUint64FromPtr(size) - data := allocWrapper(u64Size) - _ = memcpy(data, uintptr(unsafe.Pointer(input)), u64Size) + _ = memcpyGenericPowerShellObject(data, input, size) + + return data +} + +func mallocCopyStr(str string) uintptr { + + size := 2 * uint64((len(str) + 1)) + data, err := allocWrapper(size) + if err != nil { + panic("Couldn't allocate memory") + } + // safe usage due to data being c pointer + _ = memcpyStr(data, str) return data } @@ -33,30 +63,15 @@ func wsclen(str uintptr) uint64 { var charCode uint16 = 1 var i uint64 = 0 for ; charCode != 0; i++ { - charCode = *(*uint16)(unsafe.Pointer(str + (makeUintptrFromUint64(i) * unsafe.Sizeof(charCode)))) + charCode = *((*uint16)(unsafe.Pointer(str + (makeUintptrFromUint64(i) * unsafe.Sizeof(charCode))))) } return i } -func makeString(str uintptr) string { - count := wsclen(str) + 1 - arr := make([]uint16, count) - ptrwchar := unsafe.Pointer(&arr[0]) - - memcpy(uintptr(ptrwchar), str, count*2) - - s := windows.UTF16ToString(arr) - return s -} - func uintptrMakeString(ptr uintptr) string { - return makeString(ptr) + return cstrToStr(ptr) } func makeCStringUintptr(str string) uintptr { - cs, _ := windows.UTF16PtrFromString(str) - ptrwchar := unsafe.Pointer(cs) - size := 2 * (wsclen(uintptr(ptrwchar)) + 1) - - return mallocCopy(uintptr(ptrwchar), makeUintptrFromUint64(size)) + return mallocCopyStr(str) } diff --git a/pkg/powershell/hostcommand.go b/pkg/powershell/hostcommand.go index cd5d6b6..b37d45d 100644 --- a/pkg/powershell/hostcommand.go +++ b/pkg/powershell/hostcommand.go @@ -1,7 +1,5 @@ package powershell -import "unsafe" - // CallbackResultsWriter allows you to write values to powershell when inside Send-HostCommand type CallbackResultsWriter interface { WriteString(string) @@ -57,20 +55,13 @@ func (writer *callbackResultsWriter) Write(handle Object, needsClose bool) { writer.objects = append(writer.objects, obj) } -func mallocCopyGenericPowerShellObject(input *nativePowerShell_GenericPowerShellObject, inputCount uint64) *nativePowerShell_GenericPowerShellObject { - - size := uintptr(inputCount) * unsafe.Sizeof(*input) - - return (*nativePowerShell_GenericPowerShellObject)(unsafe.Pointer(mallocCopy(uintptr(unsafe.Pointer(input)), size))) -} - // filloutResults takes accumulated objects from Write calls and prepares them to cross the C boundary func (writer *callbackResultsWriter) filloutResults(res uintptr) { - results := (*nativePowerShell_JsonReturnValues)(unsafe.Pointer(res)) - results.objects = nil - results.count = 0 + var results nativePowerShell_JsonReturnValues + if writer.objects != nil && len(writer.objects) > 0 { results.count = uint32(len(writer.objects)) results.objects = mallocCopyGenericPowerShellObject(&writer.objects[0], uint64(len(writer.objects))) } + _ = memcpyJsonReturnValues(res, results) } diff --git a/pkg/powershell/powershell.go b/pkg/powershell/powershell.go index d6cbd08..12fac7a 100644 --- a/pkg/powershell/powershell.go +++ b/pkg/powershell/powershell.go @@ -118,9 +118,8 @@ func makePowerShellObjectIndexed(objects uintptr, index uint32) Object { // I don't get why I have to use unsafe.Pointer on C memory var handle nativePowerShell_PowerShellObject - ptr := unsafe.Pointer(objects) offset := (uintptr(index) * unsafe.Sizeof(handle)) - handle = *(*nativePowerShell_PowerShellObject)(unsafe.Pointer(uintptr(ptr) + offset)) + handle = *(*nativePowerShell_PowerShellObject)(unsafe.Pointer(objects + offset)) return makePowerShellObject(handle) } diff --git a/pkg/powershell/psh_host.go b/pkg/powershell/psh_host.go index 3342566..738ddca 100644 --- a/pkg/powershell/psh_host.go +++ b/pkg/powershell/psh_host.go @@ -46,7 +46,7 @@ type ( } nativePowerShell_JsonReturnValues struct { - objects *nativePowerShell_GenericPowerShellObject + objects uintptr // *nativePowerShell_GenericPowerShellObject count uint32 } nativePowerShell_ReceiveJsonCommand func(context uintptr, command nativePowerShell_StringPtr, inputrs *nativePowerShell_PowerShellObject, inputCount uint64, returnValues *nativePowerShell_JsonReturnValues) uintptr diff --git a/pkg/powershell/runspace_test.go b/pkg/powershell/runspace_test.go index 7d49051..dc58f8e 100644 --- a/pkg/powershell/runspace_test.go +++ b/pkg/powershell/runspace_test.go @@ -183,7 +183,7 @@ func ExampleCallbackHolder() { func ExampleRunspace_customSimpleLogger() { // create a custom logger object - customLogger := logger.SimpleFuncPtr{func(str string) { + customLogger := logger.SimpleFuncPtr{FuncPtr: func(str string) { fmt.Print("Custom: " + str) }} // create a runspace (where you run your powershell statements in) diff --git a/pkg/powershell/runspacehelpers.go b/pkg/powershell/runspacehelpers.go index 6dec93d..fa933c1 100644 --- a/pkg/powershell/runspacehelpers.go +++ b/pkg/powershell/runspacehelpers.go @@ -2,7 +2,6 @@ package powershell import ( "syscall" - "unsafe" ) func loggerCallback(context uint64, str uintptr) uintptr { @@ -133,15 +132,19 @@ var ( LogVerboseLine: syscall.NewCallbackCDecl(loggerCallbackVerboseln), LogDebugLine: syscall.NewCallbackCDecl(loggerCallbackDebugln), } - loggerCallbackPointer unsafe.Pointer = unsafe.Pointer(&loggerCallbackHolder) - commandCallbackPointer uintptr = syscall.NewCallbackCDecl(commandCallback) + + // intentionally leak loggerCallbackPointer (once per process) + // doing this so the callback object will be around for lifetime of process + // If it was golang object just marshalled for duration of call, golang garbage collection could move things and we would crash + loggerCallbackPointer uintptr = mallocCopyLogStringHolder(loggerCallbackHolder) + commandCallbackPointer uintptr = syscall.NewCallbackCDecl(commandCallback) ) func createRunspaceHelper(context uint64, useLogger byte, useCommand byte) nativePowerShell_RunspaceHandle { var commandPtr uintptr = 0 var loggerPtr uintptr = 0 if useLogger != 0 { - loggerPtr = uintptr(loggerCallbackPointer) + loggerPtr = loggerCallbackPointer } if useCommand != 0 { commandPtr = commandCallbackPointer @@ -152,7 +155,7 @@ func createRunspaceHelper(context uint64, useLogger byte, useCommand byte) nativ func createRemoteRunspaceHelper(context uint64, useLogger byte, remoteMachine *uint16, userName *uint16, password *uint16) nativePowerShell_RunspaceHandle { var loggerPtr uintptr = 0 if useLogger != 0 { - loggerPtr = uintptr(loggerCallbackPointer) + loggerPtr = loggerCallbackPointer } return nativePowerShell_CreateRemoteRunspace(makeUintptrFromUint64(context), loggerPtr, remoteMachine, userName, password) } diff --git a/pkg/powershell/zpsh_host.go b/pkg/powershell/zpsh_host.go index a355078..0adb7a5 100644 --- a/pkg/powershell/zpsh_host.go +++ b/pkg/powershell/zpsh_host.go @@ -189,9 +189,12 @@ func nativePowerShell_AddPSObjectHandle(handle nativePowerShell_PowerShellObject return } -func nativePowerShell_DefaultAlloc(size uint64) (status uintptr) { - r0, _, _ := syscall.Syscall(procnativePowerShell_DefaultAlloc.Addr(), 1, uintptr(size), 0, 0) +func nativePowerShell_DefaultAlloc(size uint64) (status uintptr, err error) { + r0, _, err := syscall.Syscall(procnativePowerShell_DefaultAlloc.Addr(), 1, uintptr(size), 0, 0) status = uintptr(r0) + if status != uintptr(0) { + err = nil + } return } @@ -201,7 +204,47 @@ func nativePowerShell_DefaultFree(address uintptr) { } func memcpy(dest uintptr, src uintptr, size uint64) (ptr uintptr) { - r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, uintptr(dest), uintptr(src), uintptr(size)) + r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, dest, src, uintptr(size)) ptr = uintptr(r0) return } + +func memcpyLogStringHolder(dest uintptr, src nativePowerShell_LogString_Holder) (ptr uintptr) { + + r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, dest, uintptr(unsafe.Pointer(&src)), uintptr(unsafe.Sizeof(src))) + ptr = uintptr(r0) + return +} +func memcpyJsonReturnValues(dest uintptr, src nativePowerShell_JsonReturnValues) (ptr uintptr) { + + r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, dest, uintptr(unsafe.Pointer(&src)), uintptr(unsafe.Sizeof(src))) + ptr = uintptr(r0) + return +} + +func memcpyGenericPowerShellObject(dest uintptr, src *nativePowerShell_GenericPowerShellObject, size uint64) (ptr uintptr) { + + r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, dest, uintptr(unsafe.Pointer(src)), uintptr(size)) + ptr = uintptr(r0) + return +} + +func memcpyStr(dest uintptr, str string) (ptr uintptr) { + cs, _ := windows.UTF16PtrFromString(str) + size := 2 * uint64((len(str) + 1)) + + r0, _, _ := syscall.Syscall(procmemcpy.Addr(), 3, dest, uintptr(unsafe.Pointer(cs)), uintptr(size)) + ptr = uintptr(r0) + return +} + +func cstrToStr(cstr uintptr) string { + + count := wsclen(cstr) + 1 + arr := make([]uint16, count) + + _, _, _ = syscall.Syscall(procmemcpy.Addr(), 3, uintptr(unsafe.Pointer(&arr[0])), cstr, uintptr(count*2)) + + s := windows.UTF16ToString(arr) + return s +}