From 95e8aeaf0f49edb7a6ad095e771af9305d26cbd7 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Fri, 22 Sep 2023 07:39:29 +0930 Subject: [PATCH] winlogbeat/sys/wineventlog: fix unsafe pointer use Fix the use of pointer to uintptr conversions to comply with the unsafe.Pointer rules. In particular, the code previously was not making conversions from a *T to uintptr in the call expression as required by rule (4) Conversion of a Pointer to a uintptr when calling syscall.Syscall[1]. [1]https://pkg.go.dev/unsafe#Pointer --- CHANGELOG-developer.next.asciidoc | 1 + winlogbeat/sys/wineventlog/format_message.go | 7 +++---- winlogbeat/sys/wineventlog/renderer.go | 4 ++-- winlogbeat/sys/wineventlog/syscall_windows.go | 4 ++-- .../sys/wineventlog/wineventlog_windows.go | 18 ++++++++---------- winlogbeat/sys/wineventlog/zsyscall_windows.go | 8 ++++---- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/CHANGELOG-developer.next.asciidoc b/CHANGELOG-developer.next.asciidoc index 2d5ad19bc85..ca23b4f8670 100644 --- a/CHANGELOG-developer.next.asciidoc +++ b/CHANGELOG-developer.next.asciidoc @@ -87,6 +87,7 @@ The list below covers the major changes between 7.0.0-rc2 and main only. - Renamed an httpjson input metric to follow naming conventions. `httpjson_interval_pages_total` was renamed to `httpjson_interval_pages` because the `_total` suffix is reserved for counters. {issue}35933[35933] {pull}36169[36169] - Fixed some race conditions in tests {pull}36185[36185] - Re-enable HTTPJSON fixed flakey test. {issue}34929[34929] {pull}36525[36525] +- Make winlogbeat/sys/wineventlog follow the unsafe.Pointer rules. {pull}36650[36650] ==== Added diff --git a/winlogbeat/sys/wineventlog/format_message.go b/winlogbeat/sys/wineventlog/format_message.go index 98640ed2f2c..e6502d384fa 100644 --- a/winlogbeat/sys/wineventlog/format_message.go +++ b/winlogbeat/sys/wineventlog/format_message.go @@ -21,7 +21,6 @@ package wineventlog import ( "fmt" - "unsafe" "golang.org/x/sys/windows" @@ -70,10 +69,10 @@ func getEventXML(metadata *PublisherMetadata, eventHandle EvtHandle) (string, er func evtFormatMessage(metadataHandle EvtHandle, eventHandle EvtHandle, messageID uint32, values []EvtVariant, messageFlag EvtFormatMessageFlag) (string, error) { var ( valuesCount = uint32(len(values)) - valuesPtr uintptr + valuesPtr *EvtVariant ) - if len(values) > 0 { - valuesPtr = uintptr(unsafe.Pointer(&values[0])) + if len(values) != 0 { + valuesPtr = &values[0] } // Determine the buffer size needed (given in WCHARs). diff --git a/winlogbeat/sys/wineventlog/renderer.go b/winlogbeat/sys/wineventlog/renderer.go index bd458f6d998..4471b7b6aff 100644 --- a/winlogbeat/sys/wineventlog/renderer.go +++ b/winlogbeat/sys/wineventlog/renderer.go @@ -53,12 +53,12 @@ type Renderer struct { // NewRenderer returns a new Renderer. func NewRenderer(session EvtHandle, log *logp.Logger) (*Renderer, error) { - systemContext, err := _EvtCreateRenderContext(0, 0, EvtRenderContextSystem) + systemContext, err := _EvtCreateRenderContext(0, nil, EvtRenderContextSystem) if err != nil { return nil, fmt.Errorf("failed in EvtCreateRenderContext for system context: %w", err) } - userContext, err := _EvtCreateRenderContext(0, 0, EvtRenderContextUser) + userContext, err := _EvtCreateRenderContext(0, nil, EvtRenderContextUser) if err != nil { return nil, fmt.Errorf("failed in EvtCreateRenderContext for user context: %w", err) } diff --git a/winlogbeat/sys/wineventlog/syscall_windows.go b/winlogbeat/sys/wineventlog/syscall_windows.go index f18bbeb2087..14cbf560e6f 100644 --- a/winlogbeat/sys/wineventlog/syscall_windows.go +++ b/winlogbeat/sys/wineventlog/syscall_windows.go @@ -649,14 +649,14 @@ func EvtClearLog(session EvtHandle, channelPath string, targetFilePath string) e //sys _EvtSubscribe(session EvtHandle, signalEvent uintptr, channelPath *uint16, query *uint16, bookmark EvtHandle, context uintptr, callback syscall.Handle, flags EvtSubscribeFlag) (handle EvtHandle, err error) = wevtapi.EvtSubscribe //sys _EvtCreateBookmark(bookmarkXML *uint16) (handle EvtHandle, err error) = wevtapi.EvtCreateBookmark //sys _EvtUpdateBookmark(bookmark EvtHandle, event EvtHandle) (err error) = wevtapi.EvtUpdateBookmark -//sys _EvtCreateRenderContext(ValuePathsCount uint32, valuePaths uintptr, flags EvtRenderContextFlag) (handle EvtHandle, err error) = wevtapi.EvtCreateRenderContext +//sys _EvtCreateRenderContext(ValuePathsCount uint32, valuePaths **uint16, flags EvtRenderContextFlag) (handle EvtHandle, err error) = wevtapi.EvtCreateRenderContext //sys _EvtRender(context EvtHandle, fragment EvtHandle, flags EvtRenderFlag, bufferSize uint32, buffer *byte, bufferUsed *uint32, propertyCount *uint32) (err error) = wevtapi.EvtRender //sys _EvtClose(object EvtHandle) (err error) = wevtapi.EvtClose //sys _EvtSeek(resultSet EvtHandle, position int64, bookmark EvtHandle, timeout uint32, flags uint32) (success bool, err error) [!success] = wevtapi.EvtSeek //sys _EvtNext(resultSet EvtHandle, eventArraySize uint32, eventArray *EvtHandle, timeout uint32, flags uint32, numReturned *uint32) (err error) = wevtapi.EvtNext //sys _EvtOpenChannelEnum(session EvtHandle, flags uint32) (handle EvtHandle, err error) = wevtapi.EvtOpenChannelEnum //sys _EvtNextChannelPath(channelEnum EvtHandle, channelPathBufferSize uint32, channelPathBuffer *uint16, channelPathBufferUsed *uint32) (err error) = wevtapi.EvtNextChannelPath -//sys _EvtFormatMessage(publisherMetadata EvtHandle, event EvtHandle, messageID uint32, valueCount uint32, values uintptr, flags EvtFormatMessageFlag, bufferSize uint32, buffer *byte, bufferUsed *uint32) (err error) = wevtapi.EvtFormatMessage +//sys _EvtFormatMessage(publisherMetadata EvtHandle, event EvtHandle, messageID uint32, valueCount uint32, values *EvtVariant, flags EvtFormatMessageFlag, bufferSize uint32, buffer *byte, bufferUsed *uint32) (err error) = wevtapi.EvtFormatMessage //sys _EvtOpenPublisherMetadata(session EvtHandle, publisherIdentity *uint16, logFilePath *uint16, locale uint32, flags uint32) (handle EvtHandle, err error) = wevtapi.EvtOpenPublisherMetadata //sys _EvtGetPublisherMetadataProperty(publisherMetadata EvtHandle, propertyID EvtPublisherMetadataPropertyID, flags uint32, bufferSize uint32, variant *EvtVariant, bufferUsed *uint32) (err error) = wevtapi.EvtGetPublisherMetadataProperty //sys _EvtGetEventMetadataProperty(eventMetadata EvtHandle, propertyID EvtEventMetadataPropertyID, flags uint32, bufferSize uint32, variant *EvtVariant, bufferUsed *uint32) (err error) = wevtapi.EvtGetEventMetadataProperty diff --git a/winlogbeat/sys/wineventlog/wineventlog_windows.go b/winlogbeat/sys/wineventlog/wineventlog_windows.go index 545b7940878..6b4abfaf5d1 100644 --- a/winlogbeat/sys/wineventlog/wineventlog_windows.go +++ b/winlogbeat/sys/wineventlog/wineventlog_windows.go @@ -333,19 +333,17 @@ func CreateBookmarkFromXML(bookmarkXML string) (EvtHandle, error) { // CreateRenderContext creates a render context. Close must be called on // returned EvtHandle when finished with the handle. func CreateRenderContext(valuePaths []string, flag EvtRenderContextFlag) (EvtHandle, error) { - paths := make([]uintptr, 0, len(valuePaths)) + paths := make([]*uint16, 0, len(valuePaths)) for _, path := range valuePaths { - utf16, err := syscall.UTF16FromString(path) + utf16, err := syscall.UTF16PtrFromString(path) if err != nil { return 0, err } - - paths = append(paths, reflect.ValueOf(&utf16[0]).Pointer()) + paths = append(paths, utf16) } - - var pathsAddr uintptr - if len(paths) > 0 { - pathsAddr = reflect.ValueOf(&paths[0]).Pointer() + var pathsAddr **uint16 + if len(paths) != 0 { + pathsAddr = &paths[0] } context, err := _EvtCreateRenderContext(uint32(len(paths)), pathsAddr, flag) @@ -409,7 +407,7 @@ func FormatEventString( // Determine the buffer size needed (given in WCHARs). var bufferUsed uint32 - err := _EvtFormatMessage(ph, eventHandle, 0, 0, 0, messageFlag, 0, nil, &bufferUsed) + err := _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, 0, nil, &bufferUsed) if err != windows.ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // This is an errno. return fmt.Errorf("failed in EvtFormatMessage: %w", err) } @@ -423,7 +421,7 @@ func FormatEventString( // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage bb.Reserve(int(bufferUsed * 2)) - err = _EvtFormatMessage(ph, eventHandle, 0, 0, 0, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed) + err = _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed) if err != nil { return fmt.Errorf("failed in EvtFormatMessage: %w", err) } diff --git a/winlogbeat/sys/wineventlog/zsyscall_windows.go b/winlogbeat/sys/wineventlog/zsyscall_windows.go index 62e455f09a0..3cf625d77c1 100644 --- a/winlogbeat/sys/wineventlog/zsyscall_windows.go +++ b/winlogbeat/sys/wineventlog/zsyscall_windows.go @@ -119,8 +119,8 @@ func _EvtCreateBookmark(bookmarkXML *uint16) (handle EvtHandle, err error) { return } -func _EvtCreateRenderContext(ValuePathsCount uint32, valuePaths uintptr, flags EvtRenderContextFlag) (handle EvtHandle, err error) { - r0, _, e1 := syscall.Syscall(procEvtCreateRenderContext.Addr(), 3, uintptr(ValuePathsCount), uintptr(valuePaths), uintptr(flags)) +func _EvtCreateRenderContext(ValuePathsCount uint32, valuePaths **uint16, flags EvtRenderContextFlag) (handle EvtHandle, err error) { + r0, _, e1 := syscall.Syscall(procEvtCreateRenderContext.Addr(), 3, uintptr(ValuePathsCount), uintptr(unsafe.Pointer(valuePaths)), uintptr(flags)) handle = EvtHandle(r0) if handle == 0 { err = errnoErr(e1) @@ -128,8 +128,8 @@ func _EvtCreateRenderContext(ValuePathsCount uint32, valuePaths uintptr, flags E return } -func _EvtFormatMessage(publisherMetadata EvtHandle, event EvtHandle, messageID uint32, valueCount uint32, values uintptr, flags EvtFormatMessageFlag, bufferSize uint32, buffer *byte, bufferUsed *uint32) (err error) { - r1, _, e1 := syscall.Syscall9(procEvtFormatMessage.Addr(), 9, uintptr(publisherMetadata), uintptr(event), uintptr(messageID), uintptr(valueCount), uintptr(values), uintptr(flags), uintptr(bufferSize), uintptr(unsafe.Pointer(buffer)), uintptr(unsafe.Pointer(bufferUsed))) +func _EvtFormatMessage(publisherMetadata EvtHandle, event EvtHandle, messageID uint32, valueCount uint32, values *EvtVariant, flags EvtFormatMessageFlag, bufferSize uint32, buffer *byte, bufferUsed *uint32) (err error) { + r1, _, e1 := syscall.Syscall9(procEvtFormatMessage.Addr(), 9, uintptr(publisherMetadata), uintptr(event), uintptr(messageID), uintptr(valueCount), uintptr(unsafe.Pointer(values)), uintptr(flags), uintptr(bufferSize), uintptr(unsafe.Pointer(buffer)), uintptr(unsafe.Pointer(bufferUsed))) if r1 == 0 { err = errnoErr(e1) }