Skip to content

Commit

Permalink
winlogbeat/sys/wineventlog: fix unsafe pointer use (#36650)
Browse files Browse the repository at this point in the history
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 0ad4264)

# Conflicts:
#	winlogbeat/sys/wineventlog/wineventlog_windows.go
  • Loading branch information
efd6 authored and mergify[bot] committed Sep 22, 2023
1 parent 7581e18 commit 1be29f8
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 20 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG-developer.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions winlogbeat/sys/wineventlog/format_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package wineventlog

import (
"fmt"
"unsafe"

"golang.org/x/sys/windows"

Expand Down Expand Up @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions winlogbeat/sys/wineventlog/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions winlogbeat/sys/wineventlog/syscall_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 24 additions & 8 deletions winlogbeat/sys/wineventlog/wineventlog_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -412,6 +410,7 @@ func FormatEventString(

// Create a buffer if one was not provided.
var bufferUsed uint32
<<<<<<< HEAD

Check failure on line 413 in winlogbeat/sys/wineventlog/wineventlog_windows.go

View workflow job for this annotation

GitHub Actions / lint (windows)

expected statement, found '<<' (typecheck)
if buffer == nil {
err := _EvtFormatMessage(ph, eventHandle, 0, 0, 0, messageFlag,
0, nil, &bufferUsed)
Expand All @@ -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)}
}
=======

Check failure on line 432 in winlogbeat/sys/wineventlog/wineventlog_windows.go

View workflow job for this annotation

GitHub Actions / lint (windows)

expected statement, found '==' (typecheck)
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))

Check failure on line 448 in winlogbeat/sys/wineventlog/wineventlog_windows.go

View workflow job for this annotation

GitHub Actions / lint (windows)

expected statement, found '>>' (typecheck)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions winlogbeat/sys/wineventlog/zsyscall_windows.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1be29f8

Please sign in to comment.