Skip to content

Commit

Permalink
[receiver/windowseventlog] Fix buffer overflow when reading raw events (
Browse files Browse the repository at this point in the history
open-telemetry#23678)

**Description:**
Fixing a bug where an event larger than half the buffer capacity would
cause a panic due to an overflow. The reason this happens is because raw
events are render using the `evtRender` syscall, which returns the byte
count, rather than the UTF16 character count, but we treat it as if it
was the latter.

**Link to tracking Issue:** open-telemetry#23677

**Testing:** 
Added some tests for previously untested parts of the code, and a test
that triggers the problem in particular. I've also done a E2E test on a
live system which previously triggered the problem, but doesn't after
the fix.
  • Loading branch information
Mikołaj Świątek committed Jun 28, 2023
1 parent 207b866 commit 37a919c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 1 deletion.
20 changes: 20 additions & 0 deletions .chloggen/fix_stanza_windows-buffer.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Use this changelog template to create an entry for release notes.
# If your change doesn't affect end users, such as a test fix or a tooling change,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: windowseventlogreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix buffer overflow when ingesting large raw Events

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [23677]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
2 changes: 1 addition & 1 deletion pkg/stanza/operator/input/windows/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (e *Event) RenderRaw(buffer Buffer) (EventRaw, error) {
buffer.UpdateSizeBytes(*bufferUsed)
return e.RenderRaw(buffer)
}
bytes, err := buffer.ReadWideChars(*bufferUsed)
bytes, err := buffer.ReadBytes(*bufferUsed)
if err != nil {
return EventRaw{}, fmt.Errorf("failed to read bytes from buffer: %w", err)
}
Expand Down
53 changes: 53 additions & 0 deletions receiver/windowseventlogreceiver/receiver_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package windowseventlogreceiver

import (
"context"
"encoding/xml"
"path/filepath"
"testing"
"time"
Expand Down Expand Up @@ -126,6 +127,58 @@ func TestReadWindowsEventLogger(t *testing.T) {
require.Equal(t, int64(10), eventIDMap["id"])
}

func TestReadWindowsEventLoggerRaw(t *testing.T) {
logMessage := "Test log"

ctx := context.Background()
factory := NewFactory()
createSettings := receivertest.NewNopCreateSettings()
cfg := createTestConfig()
cfg.InputConfig.Raw = true
sink := new(consumertest.LogsSink)

receiver, err := factory.CreateLogsReceiver(ctx, createSettings, cfg, sink)
require.NoError(t, err)

err = receiver.Start(ctx, componenttest.NewNopHost())
require.NoError(t, err)
defer receiver.Shutdown(ctx)

src := "otel"
err = eventlog.InstallAsEventCreate(src, eventlog.Info|eventlog.Warning|eventlog.Error)
defer eventlog.Remove(src)
require.NoError(t, err)

logger, err := eventlog.Open(src)
require.NoError(t, err)
defer logger.Close()

err = logger.Info(10, logMessage)
require.NoError(t, err)

logsReceived := func() bool {
return sink.LogRecordCount() == 1
}

// logs sometimes take a while to be written, so a substantial wait buffer is needed
require.Eventually(t, logsReceived, 10*time.Second, 200*time.Millisecond)
results := sink.AllLogs()
require.Len(t, results, 1)

records := results[0].ResourceLogs().At(0).ScopeLogs().At(0).LogRecords()
require.Equal(t, 1, records.Len())

record := records.At(0)
body := record.Body().AsString()
bodyStruct := struct {
Data string `xml:"EventData>Data"`
}{}
err = xml.Unmarshal([]byte(body), &bodyStruct)
require.NoError(t, err)

require.Equal(t, logMessage, bodyStruct.Data)
}

func createTestConfig() *WindowsLogConfig {
return &WindowsLogConfig{
BaseConfig: adapter.BaseConfig{
Expand Down

0 comments on commit 37a919c

Please sign in to comment.