From 1be29f88168d8fe969bd3c1fc446c238ff85ccaa Mon Sep 17 00:00:00 2001 From: Dan Kortschak <90160302+efd6@users.noreply.github.com> Date: Sat, 23 Sep 2023 06:08:51 +0930 Subject: [PATCH] winlogbeat/sys/wineventlog: fix unsafe pointer use (#36650) 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 (cherry picked from commit 0ad4264557759c192ac3560f7b8bba96329ba936) # Conflicts: # winlogbeat/sys/wineventlog/wineventlog_windows.go --- CHANGELOG-developer.next.asciidoc | 13 ++++++++ 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 | 32 ++++++++++++++----- .../sys/wineventlog/zsyscall_windows.go | 8 ++--- 6 files changed, 48 insertions(+), 20 deletions(-) diff --git a/CHANGELOG-developer.next.asciidoc b/CHANGELOG-developer.next.asciidoc index 3cdd2100bc8..db50dc660e6 100644 --- a/CHANGELOG-developer.next.asciidoc +++ b/CHANGELOG-developer.next.asciidoc @@ -62,6 +62,19 @@ The list below covers the major changes between 7.0.0-rc2 and master only. - Errors should be thrown as errors. Metricsets inside Metricbeat will now throw errors as the `error` log level. {pull}27804[27804] - Avoid panicking in `add_fields` processor when input event.Fields is a nil map. {pull}28219[28219] - Fix type mismatch in libbeat/metric/system/cgroup/cgv2 when building on mips platforms. {pull}34658[34658] +- Drop event batch when get HTTP status 413 from Elasticsearch to avoid infinite loop {issue}14350[14350] {pull}29368[29368] +- Allow to use metricbeat for named mssql instances. {issue}24076[24076] {pull}30859[30859] +- Setting DEV=true when running `mage build` now correctly generates binaries without optimisations and with debug symbols {pull}31955[31955] +- The beat.cgroup.memory.mem.usage.bytes metric is now a gauge {issue}31582[31582] {pull}32652[32652] +- Fix the integration testcase docker port mapping for sql and oracle modules {pull}34221[34221] +- Fix the ingest pipeline for mysql slowlog to parse schema name with dash {pull}34371[34372] +- Fix the multiple host support for mongodb module {pull}34624[34624] +- Skip HTTPJSON flakey test. {issue}34929[34929] {pull}35138[35138] +- Fix ingest pipeline for panw module to parse url scheme correctly {pull}35757[35757] +- 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 f97024b663c..4e89022eeb0 100644 --- a/winlogbeat/sys/wineventlog/format_message.go +++ b/winlogbeat/sys/wineventlog/format_message.go @@ -22,7 +22,6 @@ package wineventlog import ( "fmt" - "unsafe" "golang.org/x/sys/windows" @@ -71,10 +70,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 310eab450f6..d03afd1c74e 100644 --- a/winlogbeat/sys/wineventlog/renderer.go +++ b/winlogbeat/sys/wineventlog/renderer.go @@ -60,12 +60,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 ffa7a2ae150..323ce6a0605 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) @@ -412,6 +410,7 @@ func FormatEventString( // Create a buffer if one was not provided. var bufferUsed uint32 +<<<<<<< HEAD if buffer == nil { err := _EvtFormatMessage(ph, eventHandle, 0, 0, 0, messageFlag, 0, nil, &bufferUsed) @@ -430,6 +429,23 @@ func FormatEventString( if err == ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // This is an errno or nil. return sys.InsufficientBufferError{Cause: err, RequiredSize: int(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) + } + + // Get a buffer from the pool and adjust its length. + bb := sys.NewPooledByteBuffer() + defer bb.Free() + // The documentation for EvtFormatMessage specifies that the buffer is + // requested "in characters", and the buffer itself is LPWSTR, meaning the + // characters are WCHAR so double the value. + // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage + bb.Reserve(int(bufferUsed * 2)) + + err = _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed) +>>>>>>> 0ad4264557 (winlogbeat/sys/wineventlog: fix unsafe pointer use (#36650)) if err != nil { return 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) }